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(semantic-commit-linting): add semantic commit linting #77

Merged
merged 1 commit into from
Dec 4, 2017

Conversation

jaebradley
Copy link
Contributor

@jaebradley jaebradley commented Nov 17, 2017

The commitmsg hook fails when you try and add a commit message via git commit that does not follow conventional commit convention

image

@jaebradley jaebradley force-pushed the valid-commit-message-hook branch 2 times, most recently from 720a82d to 9260f66 Compare November 17, 2017 19:25
@jaebradley jaebradley self-assigned this Nov 17, 2017
@jaebradley jaebradley added the enhancement Relates to new features or improvements to existing features label Nov 17, 2017
@arizzitano
Copy link
Contributor

This is a valuable bit of automation, but IMO too heavyweight to run for every commit. Some developers (myself included) push up a lot of WIP stuff as they're working on it, just so others can see it or CI can run against it, and those commit messages are generally meaningless. Besides, people tend to get antsy when you prevent them from committing -- even the pre-commit linter annoys some folks. Why enforce conventional commit syntax when it doesn't have any deeper implications?

The only commit message that really matters is the squash commit, right? What do you think about using the prepare-commit-msg hook instead, and only running it if the hook receives the squash param?

@jaebradley
Copy link
Contributor Author

jaebradley commented Nov 20, 2017

@arizzitano that all makes sense - I guess you could still commit using the --no-verify option? Either way, I don't really have a preference!

@jaebradley jaebradley force-pushed the valid-commit-message-hook branch 2 times, most recently from e34376b to 5c6b3a4 Compare November 29, 2017 22:56
fix(package.json): change commit script

chore(dependencies): add back package-lock.json

added some packages

updated package.json

added prepare commit message git hook

chore(scripts): update scripts

chore(eslint): remove unused eslintignore value

chore(eslintignore): remove unnecessary value

chore(package-scripts): used old script name
@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Coverage remained the same at 99.505% when pulling 5c6b3a4 on valid-commit-message-hook into 25fe397 on master.

@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Coverage remained the same at 99.501% when pulling 720a82d on valid-commit-message-hook into 9690fda on master.

@openedx openedx deleted a comment from coveralls Nov 29, 2017
@openedx openedx deleted a comment from coveralls Nov 29, 2017
@openedx openedx deleted a comment from coveralls Nov 29, 2017
@openedx openedx deleted a comment from coveralls Nov 29, 2017
@openedx openedx deleted a comment from coveralls Nov 29, 2017
Copy link
Contributor

@arizzitano arizzitano left a comment

Choose a reason for hiding this comment

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

Let's give this a try. If people find it too heavyweight we can always revert.

@jaebradley jaebradley merged commit 57c13c4 into master Dec 4, 2017
@jaebradley jaebradley deleted the valid-commit-message-hook branch December 4, 2017 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Relates to new features or improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants