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

Added precommit hook #67

Merged
merged 4 commits into from
Apr 25, 2021
Merged

Added precommit hook #67

merged 4 commits into from
Apr 25, 2021

Conversation

Samridhi-98
Copy link
Contributor

Description

Added pre-commit hook .pre-commit-config.yaml

Fixes #62

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Samridhi-98
Copy link
Contributor Author

@Manvityagi Please review my PR.

@Manvityagi Manvityagi requested a review from abdus April 24, 2021 14:07
@abdus
Copy link
Member

abdus commented Apr 24, 2021

hey @Samridhi-98, did you test these changes locally before making a Pull Request? from what I can see, you have installed husky@6 for this project. But the way you have implemented it .. works with husky@4. so, the pre-commit hooks never actually get triggered.

following are the changes I want you to make:

  1. remove `.pre-commit-config.yaml. this is required only if you are using pre-commit framework
  2. migrate the implementation to husky@6. resources: installation · migration
  3. update readme with relevant information (for example, if there's any additional commands user required to run while setting up the project locally)

Let me know if you need any help. And please test your code locally before making PRs.

@Samridhi-98
Copy link
Contributor Author

hey @Samridhi-98, did you test these changes locally before making a Pull Request? from what I can see, you have installed husky@6 for this project. But the way you have implemented it .. works with husky@4. so, the pre-commit hooks never actually get triggered.

following are the changes I want you to make:

  1. remove `.pre-commit-config.yaml. this is required only if you are using pre-commit framework
  2. migrate the implementation to husky@6. resources: installation · migration
  3. update readme with relevant information (for example, if there's any additional commands user required to run while setting up the project locally)

Let me know if you need any help. And please test your code locally before making PRs.

Okay, will update as you suggested.

@Samridhi-98
Copy link
Contributor Author

@abdus Please review I have updated as you suggested and also tested my code locally.

Screenshot

ss

Copy link
Member

@abdus abdus left a comment

Choose a reason for hiding this comment

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

The changes you have made aren't working as expected. Files that were changed since the last commit, should be formatted automatically using prettier before commit.

Also, by "test your code locally", I meant to say that whatever code you have changed, is working as expected. For example, here you are working on formatting code with the pre-commit hook. check that whether the changed files were formatted or not.

I wrote a how-to on writing a pre-commit hook using husky@6. please check it out and see if this could be of any help.

README.md Outdated
Comment on lines 34 to 36
# To test run pre commit hook
$ npm test

Copy link
Member

Choose a reason for hiding this comment

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

pre-commit hook should run automatically before committing the changes.

package.json Outdated
Comment on lines 36 to 42
"devDependencies": {
"chai": "^4.3.4",
"faker": "^5.5.2",
"husky": "^6.0.0",
"mocha": "^8.3.2",
"sinon": "^10.0.0"
}
Copy link
Member

Choose a reason for hiding this comment

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

you need to install prettier and pretty-quick as well.

@Samridhi-98
Copy link
Contributor Author

Samridhi-98 commented Apr 25, 2021

@abdus Please review. Also while running this npx husky add .husky/pre-commit './node_modules/.bin/pretty-quick --staged' got an error so I have created the file using npx husky add .husky/pre-commit first and then added this './node_modules/.bin/pretty-quick --staged' manually and its working, please let me know if I did anything wrong.

Screenshot
github

@Samridhi-98 Samridhi-98 requested a review from abdus April 25, 2021 07:46
@abdus abdus merged commit fe446cf into Girl-Code-It:develop Apr 25, 2021
@Samridhi-98
Copy link
Contributor Author

Samridhi-98 commented Apr 25, 2021

@abdus @Manvityagi please add the appropriate label and GSSOC tag.

@abdus abdus added GSSOC21 for GSSoC participants LEVEL1 labels Apr 26, 2021
Samridhi-98 added a commit to Samridhi-98/Opportunity-Calendar-Backend that referenced this pull request May 2, 2021
* Added precommit hook

* Updated and tested husky

* added pre-commit and updated package.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSSOC21 for GSSoC participants LEVEL1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add precommit Hook for code formatting
2 participants