Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: overall structure and reliability of the function #1288

Closed

Conversation

thebedigupta
Copy link
Contributor

Description
In this PR I worked on that issue
This is the test results that I performed test-results.txt

Resolve

  • The function now correctly uses Promises without mixing in async/await
  • It still performs the same core functionality of walking through the hooks directory and registering each hook file,but in a more error resistant way
  • lead to better performance and easier-to-understand control flow.

Copy link

changeset-bot bot commented Oct 7, 2024

⚠️ No Changeset found

Latest commit: cda6448

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@thebedigupta thebedigupta changed the title fix/overall structure and reliability of the function. fix:overall structure and reliability of the function. Oct 7, 2024
@thebedigupta thebedigupta changed the title fix:overall structure and reliability of the function. fix: overall structure and reliability of the function Oct 7, 2024
These are not necessary changes that I make in this PR It was just a part of confusion that I make because this was the first time when I am contributing to any open source project
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why there are so many changes in this file, can you please explain?

why function is now sync? wasn't it enough to just remove async from promise?

…roach

sorry for over complementing things you are right this was the straight-forward approach for solving this issue
Changes made:
- Removed 'async' keyword from the Promise executor function in registerLocalHooks
- No other modifications were made to preserve the original functionality
@thebedigupta
Copy link
Contributor Author

why there are so many changes in this file, can you please explain?

why function is now sync? wasn't it enough to just remove async from promise?

sorry for overcomplicating the work you are right removing async is the straight-forward approach

remove unwanted comments and add space
Copy link

sonarqubecloud bot commented Oct 9, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants