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

Commit message linter only checks top-most commit in a git tree #309

Closed
joshuagl opened this issue Jun 12, 2019 · 1 comment · Fixed by #312
Closed

Commit message linter only checks top-most commit in a git tree #309

joshuagl opened this issue Jun 12, 2019 · 1 comment · Fixed by #312
Assignees
@rnjudge
Labels
bug Something went wrong CI/CD Continuous Integration and Continuous Delivery tests Issues related to testing the code
Milestone
Release 0.5.0

Comments

@joshuagl
Copy link
Contributor

Describe the bug
The git commit linter test_commit_message.py introduced in #302 only checks the git HEAD (usually the top-most commit in the current git branch). Therefore for multi-patch pull requests only the final patch has its commit message linted.

To Reproduce
Steps to reproduce the behavior:

  1. add a commit that violates the commit message policy to tern
  2. add a commit that complies with the commit message policy for tern
  3. run the commit message linter in the way defined in .circleci/config.yml -- python tests/test_commit_message.py
  4. observe that the linter returns without error

Expected behavior
The linter should check each commit in the PR.

A reasonable approach might be to split the linting logic into a function, compare the current branch to origin/master and run the linter against each message that exists in the current branch but not in origin/master.

The error message will need be updated, I'd like to see it report the short commit hash (git rev-parse --short COMMITSHA), and perhaps the title, of the message which triggered an error, rather than just the offending line. That would make it easier to find the offending commit in an interactive rebase.
@nishakm
Copy link
Contributor

nishakm commented Jun 12, 2019

We expected CircleCI to run the test on every commit in a PR but apparently it doesn't. So we'll put in a fix. Thanks for catching it!

@nishakm nishakm added this to the Release 0.5.0 milestone Jun 12, 2019
@nishakm nishakm added bug Something went wrong CI/CD Continuous Integration and Continuous Delivery tests Issues related to testing the code labels Jun 12, 2019
rnjudge added a commit to rnjudge/tern that referenced this issue Jun 12, 2019
@rnjudge
Make sure commit linter checks all commits
6098551 
In a previous commit (0cba95a) test_commit_message.py was introduced
under the assumption that circleci would run against all commits. We
soon realized that this was not the case and in fact we were only
checking the HEAD commit.

This commit introduces the following changes:

  1) Creates a new direrctory called 'ci'
  2) Moves test_commit_message.py from 'tests' to 'ci'
  3) Made moderate edits to test_commit_message.py in order to create a
     separate commit message linting function. Also updated the error
     messages to specify the offending commit.

Resolves tern-tools#309

Signed-off-by: Rose Judge <[email protected]>
@rnjudge rnjudge mentioned this issue Jun 12, 2019
Merged
rnjudge added a commit to rnjudge/tern that referenced this issue Jun 12, 2019
@rnjudge
Make sure commit linter checks all commits
11b740d 
In a previous commit (0cba95a) test_commit_message.py was introduced
under the assumption that circleci would run against all commits. We
soon realized that this was not the case and in fact we were only
checking the HEAD commit.

This commit introduces the following changes:

  1) Creates a new directory called 'ci'
  2) Moves test_commit_message.py from 'tests' to 'ci'
  3) Made moderate edits to test_commit_message.py in order to create a
     separate commit message linting function. Also updated the error
     messages to specify the offending commit.

Resolves tern-tools#309

Signed-off-by: Rose Judge <[email protected]>
nishakm pushed a commit that referenced this issue Jun 12, 2019
@rnjudge
Make sure commit linter checks all commits
aebecdd 
In a previous commit (0cba95a) test_commit_message.py was introduced
under the assumption that circleci would run against all commits. We
soon realized that this was not the case and in fact we were only
checking the HEAD commit.

This commit introduces the following changes:

  1) Creates a new directory called 'ci'
  2) Moves test_commit_message.py from 'tests' to 'ci'
  3) Made moderate edits to test_commit_message.py in order to create a
     separate commit message linting function. Also updated the error
     messages to specify the offending commit.

Resolves #309

Signed-off-by: Rose Judge <[email protected]>
rnjudge added a commit that referenced this issue Aug 28, 2019
@rnjudge
Make sure commit linter checks all commits
e1a02f6 
In a previous commit (0cba95a) test_commit_message.py was introduced
under the assumption that circleci would run against all commits. We
soon realized that this was not the case and in fact we were only
checking the HEAD commit.

This commit introduces the following changes:

  1) Creates a new directory called 'ci'
  2) Moves test_commit_message.py from 'tests' to 'ci'
  3) Made moderate edits to test_commit_message.py in order to create a
     separate commit message linting function. Also updated the error
     messages to specify the offending commit.

Resolves #309

Signed-off-by: Rose Judge <[email protected]>
rnjudge added a commit to rnjudge/tern that referenced this issue Jun 5, 2020
@rnjudge
Make sure commit linter checks all commits
9e958ae 
In a previous commit (0cba95a) test_commit_message.py was introduced
under the assumption that circleci would run against all commits. We
soon realized that this was not the case and in fact we were only
checking the HEAD commit.

This commit introduces the following changes:

  1) Creates a new directory called 'ci'
  2) Moves test_commit_message.py from 'tests' to 'ci'
  3) Made moderate edits to test_commit_message.py in order to create a
     separate commit message linting function. Also updated the error
     messages to specify the offending commit.

Resolves tern-tools#309

Signed-off-by: Rose Judge <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rnjudge rnjudge

Labels
bug Something went wrong CI/CD Continuous Integration and Continuous Delivery tests Issues related to testing the code
Projects
None yet
Milestone
Release 0.5.0
Development

Successfully merging a pull request may close this issue.

Make sure commit linter checks all commits rnjudge/tern
3 participants
@nishakm @rnjudge @joshuagl