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

Remove pre commit hook from package.json which generates webpack #88

Merged
merged 6 commits into from
Feb 19, 2019

Conversation

abhayks1
Copy link
Contributor

@abhayks1 abhayks1 commented Feb 14, 2019

  • Pre commit hook is there to generate webpack which generates distribution in dist directory. It should be removed.
  • Builds should not be part of the repository, but instead be done in the npm prepack script so that they are built when packaging or publishing.
    Also, a library that is published usually goes into a lib directory, not a dist directory. But that is something we also don’t do everywhere, yet. And any build artifacts should be listed in .gitignore.

Fixes #88

@abhayks1 abhayks1 self-assigned this Feb 14, 2019
Copy link
Contributor

@gulshanvasnani gulshanvasnani left a comment

Choose a reason for hiding this comment

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

Good 👍

As ignore option of lint-staged in package.json ignores linting of js files under dist directory.I think we can also remove ignore option from lint-staged.
What do you think ?

@abhayks1
Copy link
Contributor Author

Good 👍

As ignore option of lint-staged in package.json ignores linting of js files under dist directory.I think we can also remove ignore option from lint-staged.
What do you think ?

Agree! Changes has been made.

gulshanvasnani
gulshanvasnani previously approved these changes Feb 18, 2019
Copy link
Contributor

@gulshanvasnani gulshanvasnani left a comment

Choose a reason for hiding this comment

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

LGTM 👍

benjaminbollen
benjaminbollen previously approved these changes Feb 19, 2019
Copy link
Contributor

@benjaminbollen benjaminbollen left a comment

Choose a reason for hiding this comment

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

LGTM

@benjaminbollen benjaminbollen merged commit 57ecd75 into OpenST:develop Feb 19, 2019
@abhayks1 abhayks1 deleted the remove_precommit_hook branch February 26, 2019 11:58
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.

3 participants