Skip to content

chore(script): check message during commit #8

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

Merged
merged 6 commits into from
Sep 8, 2020

Conversation

anis-campos
Copy link

@anis-campos anis-campos commented Sep 2, 2020

  • Add script that checks message from file
  • Add precommit hook to help python developers make good commit messages

@anis-campos anis-campos requested review from lumautomation and a team as code owners September 2, 2020 13:35
Copy link

@noirbee noirbee left a comment

Choose a reason for hiding this comment

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

I think this makes sense only as a commit-msg hook, not a pre-commit one.

I doubt the hook configuration itself works as-is, given that AFAIK pre-commit passes files to hooks, not commit SHAs

@anis-campos anis-campos requested a review from noirbee September 2, 2020 14:09
@anis-campos anis-campos force-pushed the chore/precommit_hook branch 3 times, most recently from a72f755 to 4881008 Compare September 2, 2020 16:40
Copy link

@noirbee noirbee left a comment

Choose a reason for hiding this comment

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

See comments

@@ -242,6 +242,23 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
```

## Add pre-commit plugin
Copy link
Contributor

Choose a reason for hiding this comment

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

I try it on the repo itself with this branch as rev, but I wasn't able to make it work :(
Also I got some linting errors

Copy link
Author

Choose a reason for hiding this comment

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

I changed the readme, maybe this is the step mising in you test :
https://github.com/lumapps/commit-message-validator/blame/chore/precommit_hook/README.md#L266

Copy link
Author

@anis-campos anis-campos Sep 3, 2020

Choose a reason for hiding this comment

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

Actually without --hook-type commit-msg it can't work. I fixed the docs

- Add script that checks message from file
- Add precommit hook to help python developers make goo commit messages
@anis-campos
Copy link
Author

#8 (comment)

I'm finally keeping the echo message to prevent loss as describe inside the script.

@anis-campos
Copy link
Author

anis-campos commented Sep 3, 2020

And as discussed with @sebastien-boulle we might depreciate this in favor of a different approach:

  • Prepare the commit message based on the branch ( e.i. <type>/<jira_ticket>/<the-scope>_<the_subject>) via prepare-commit-msg hook
  • Warn of incorrect commit message ( post-commit hook)
  • Prevent push with a pre-push hook that validates all the commit messages of the branch and prints the errors of each one

But in the mean time let's keeps this, as I'm short in time right now to do all that

Copy link

@noirbee noirbee left a comment

Choose a reason for hiding this comment

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

Not much to say otherwise

check_message.sh Outdated

fi

# print message so you don't loose it in case of errors
Copy link

Choose a reason for hiding this comment

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

lose*

check_message.sh Outdated
# (in case you are not using `-m` option)
printf "checking commit message:\n\n#BEGIN#\n%s\n#END#\n\n" "$(grep -v "#" <<< "$MESSAGE")"

COMMIT_VALIDATOR_ALLOW_TEMP=1 COMMIT_VALIDATOR_NO_JIRA=1 validate "$MESSAGE"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope you don't want to deactivate JIRA validation

Anis Da Silva Campos added 2 commits September 7, 2020 22:15
Now we can fixup locally.
The github action should still block the merge though
adding the configuration necesarry to run the validator during
@sebastien-boulle sebastien-boulle force-pushed the chore/precommit_hook branch 2 times, most recently from 5eec7f2 to 3103930 Compare September 7, 2020 20:23
@anis-campos anis-campos merged commit 2b192f3 into master Sep 8, 2020
@anis-campos anis-campos deleted the chore/precommit_hook branch September 8, 2020 07:44
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