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.

Sign up for GitHub

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

Jump to bottom

Implement pre-commit hook for linting typescript_app and chaincode/typescript #329

Closed
wants to merge 2 commits into from
Closed

Implement pre-commit hook for linting typescript_app and chaincode/typescript #329

wants to merge 2 commits into from

Conversation

kunaljain0212
Copy link

This PR implements pre-commit hook using husky, which is being used for linting typescript_app and chaincode/typescript.

Closes #324

@sichen1234 sichen1234 requested a review from hs2361 October 25, 2021 20:15
@hs2361
Copy link
Contributor

hs2361 commented Oct 28, 2021

Hi, @kunaljain0212 thanks for your PR! Could you resolve the conflicts please? Also, I can see that you've added a prepare script. Does a new user need to run npm run prepare? If so, please update the docs with this step as well. Otherwise, your PR LGTM.

@kunaljain0212
Copy link
Author

Hi, @kunaljain0212 thanks for your PR! Could you resolve the conflicts please? Also, I can see that you've added a prepare script. Does a new user need to run npm run prepare? If so, please update the docs with this step as well. Otherwise, your PR LGTM.

The prepare script runs automatically after npm install, as mentioned in npm scripts documentation here - https://docs.npmjs.com/cli/v7/using-npm/scripts

hs2361
hs2361 approved these changes Nov 2, 2021
View reviewed changes
Copy link
Contributor

@hs2361 hs2361 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your PR! @udosson please take a look at this as well.

@hs2361 hs2361 requested a review from udosson November 2, 2021 14:59
@udosson
Copy link
Contributor

udosson commented Nov 3, 2021

@kunaljain0212 thanks for the PR. LGTM!!
Could you please fix the conflicts? Then I'll merge your PR.
Thanks!

@kunaljain0212
Copy link
Author

@udosson @hs2361 fixed merge conflicts 🚀

@udosson
Copy link
Contributor

udosson commented Nov 3, 2021

@kunaljain0212, thanks for resolving the conflicts. Could you squash merge your commit history into one commit, please?

hs2361
hs2361 suggested changes Nov 3, 2021
View reviewed changes
Copy link
Contributor

@hs2361 hs2361 left a comment

Choose a reason for hiding this comment

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

@kunaljain0212 Could you commit the typescript_app/package-lock.json as well? The package-lock.json is required for the npm ci command to run (this is why the lint CI is failing).

@kunaljain0212
Added pre-commit hook
5e1eb0a 
    Signed-off-by: kunaljain0212 <[email protected]>
@kunaljain0212
Copy link
Author

There were too many conflicts to resolve, I'll open a new PR for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hs2361 hs2361 hs2361 requested changes

@udosson udosson Awaiting requested review from udosson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a pre-commit hook for lint checks
3 participants
@kunaljain0212 @hs2361 @udosson