Original commit message is checked after the Pull Request commit has been updated

[gaohoward]

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

Is there an existing issue for this?

• [X] I have searched the existing issues

GitHub-hosted runner

ubuntu-latest

Additional runner information

No response

Current Behavior

I set up a commit-message-checker workflow to check the commit message of a Pull request. The pattern is to make sure the message begin with certain formatted text. Then I send a PR with a commit message that doesn't match the pattern, the workflow detects it and give failure result. Then I updated the commit's message to something that matches the pattern (locally, run git comment --amend to modify the commit message) and force pushed the commit to repository. The workflow re-triggered automatically. However it still use the old commit message and failed the workflow.

Expected Behavior

I'd expect the workflow could pick up my commit updates and give a successful workflow result.

Steps To Reproduce

1. Setup a workflow in your repository using this commit-message-checker
2. send a PR with a commit message that doesn't match the pattern in the workflow.
3. wait the workflow to fail.
4. update the commit message using git commit --amend so that the commit message matches the pattern in workflow
5. force push the commit/branch to the repo
6. wait for the workflow to be re-triggered and failed again

Anything else?

It would be always check the updated commit message rather than the original one.

[codebydant]

Hi @gaohoward,

this seems strange because I have been using this action for a long time with the exact steps you mentioned and is working fine. My guess is that you are not actually updating the last commit with the --ammend or you have a commit history in the pull request where 1 of the commits does not match the regex pattern.

[gaohoward]

Hi @dtcMLOps , I'm pretty sure I used --amend on the last commit and force pushed it to update the PR. I seems the checker keeps checking the old PR commit message (btw which appears in the title bar on the PR page and not updated even though the newest commit message has been pushed). I now use my own script to do the check, here is the code:

https://github.com/artemiscloud/activemq-artemis-operator/blob/main/.github/workflows/merge-rules.yml#L11

[rb1]

I'm also seeing this behaviour when testing this out.

• Create a PR that contains a "bad" commit message.
• use git commit --amend and force push
• Checker still picks up the old commit and fails on that

If I close the PR and recreate it then it works as expected, but closing and recreating the PR is not desirable in my use case.

[fnagel]

Same problem here. Seems updated commit messages are not checked but the original one. Closing and reopening an PR does not work... Any ideas?

Here is my workflow: https://github.com/fnagel/t3extblog/actions/runs/6869650210/job/18682867295?pr=267 https://github.com/fnagel/t3extblog/blob/65cffe03dfc06410122c38276f8f53dad748950b/.github/workflows/commit-message.yml

Works after manual merge: https://github.com/fnagel/t3extblog/actions/runs/6869676303

[codebydant]

@fnagel have you tried a different regex pattern? you can use this website https://regexr.com/ to make sure the regex pattern is working fine

[fnagel]

@dtcMLOps The pattern works fine. The actions checks an outdated commit message.

See https://github.com/fnagel/t3extblog/actions/runs/6869650210/job/18682867295?pr=267#step:2:12 (before merge in PR) https://github.com/fnagel/t3extblog/actions/runs/6869676303/job/18682954686#step:2:12 (after manual merge)

[codebydant]

@fnagel the commits before and after are not the same.

This is the commit before:

• "Add types to Content model"

This is the commit after

• "[BUGFIX] Add types to Content model"
• "[BUGFIX] Fix error in TSConfig condition"

In summary, you must be using the title of the pull request during merging as a commit message

[fnagel]

@dtcMLOps Thanks for your help!

the commits before and after are not the same.

I know, but I did not change the message when merging manually. Even the Github PR page showed the correct message (with the "[BUGFIX]..." prefix. See here: https://github.com/fnagel/t3extblog/pull/267/commits

In summary, you must be using the title of the pull request during merging as a commit message

Not sure what is meant with this. Can you please explain.

[codebydant]

@fnagel ohhh i see. You are right. I think the issue relates to how GitHub gets the HEAD when running the github actions. What I recommend you is to update the https://github.com/fnagel/t3extblog/blob/master/.github/workflows/commit-message.yml workflow to include the https://github.com/actions/checkout before running the gscommit message checker. In that way, you will receive all the commits in the pull request and then, the checker can read the commit messages in the correct way.

I have this in my workflow

In your case I think, depth = 0 is not necessary.

[fnagel]

So, this should work as expected, right? https://github.com/fnagel/t3extblog/pull/268/files

ps: fetch-depth might be expensive, see https://github.com/actions/checkout#usage

# Number of commits to fetch. 0 indicates all history for all branches and tags.
# Default: 1
fetch-depth: ''

[codebydant]

Hi @fnagel , yes that should work. I have been using this action a lot time without any issue.

[fnagel]

@dtcMLOps I've just amend and force pushed the PR with a valid new commit message, but the action still uses the initial commit message :-/

https://github.com/fnagel/t3extblog/actions/runs/6880677110/job/18715401752?pr=268

Maybe it will work only after merge? Even though the changed workflow file seems already be used.

[codebydant]

@fnagel try marking these parameters as true:

excludeTitle: true
excludeDescription: true
checkAllCommitMessages: true

and provide the accesstoken:

accessToken: ${{ secrets.GTHUB_TOKEN }}

[fnagel]

@dtcMLOps It seems to work with excludeTitle and excludeDescription only! Thanks a lot!

I did some tests, it works even though there are always two check (push and pull_request). See here: https://github.com/fnagel/t3extblog/pull/268 It's interesting that the pull_request one still has the old commit message in the title but says "No commits found in the payload, skipping check.": https://github.com/fnagel/t3extblog/actions/runs/6882923740/job/18722427585?pr=268

Sadly, it seems my line length check does no longer work: https://github.com/fnagel/t3extblog/actions/runs/6882862878/job/18722248177?pr=268#step:5:1 Pretty sure it did some time ago. It fails even when I remove the excludeTitle / excludeDescription properties from the length check: https://github.com/fnagel/t3extblog/actions/runs/6882953980/job/18722515884?pr=268

[codebydant]

Hi @fnagel I remember that the max lenght regex pattern has some errors in the provided example. Maybe you can try a different one.

This is my regex pattern for max length

# validate message length to 70 characters max.
- name: Check Subject Max Line Length
  uses: commit-message-checker@v2.0.0
  with:
    pattern: "^.{0,70}(\n.*)*$"
    error: "The maximum line length of 70 characters is exceeded."
    excludeTitle: "true" # optional: this excludes the title of a pull request
    excludeDescription: "true" # optional: this excludes the description body of a pull request
    checkAllCommitMessages: "true" # optional: this checks all commits associated with a pull request
    accessToken: ${{ secrets.GITHUB_TOKEN }} # github access token is only required if checkAllCommitMessages is true

[codebydant]

@fnagel

I did some tests, it works even though there are always two check (push and pull_request). See here: https://github.com/fnagel/t3extblog/pull/268 It's interesting that the pull_request one still has the old commit message in the title but says "No commits found in the payload, skipping check.": https://github.com/fnagel/t3extblog/actions/runs/6882923740/job/18722427585?pr=268

enable the checkAllCommits to true and provide the access token.

[fnagel]

I've just noticed that sometimes my length check works (but this is the old branch, without the latest workflow changes): https://github.com/fnagel/t3extblog/actions/runs/6883815705/job/18725113078

I need to test this a little more. Tomorrow :-) Thanks a lot for your help!

[fnagel]

@dtcMLOps Sorry for the late response. My workforce was needed elsewhere :-/

I've tried using your regex, but that didn't help, see: https://github.com/fnagel/t3extblog/actions/runs/6981695838/job/18999376712

Adding checkAllCommits seems to help with the "No commits found in the payload, skipping check." problem (see https://github.com/fnagel/t3extblog/actions/runs/6981782801/job/18999633723) but it still does not work for the length check.

Any ideas?

[codebydant]

Hi @fnagel i think here should be 0

^.{0,72}(\n.*)*$

instead 10

[fnagel]

Hi @dtcMLOps, sadly, that did not help: https://github.com/fnagel/t3extblog/actions/runs/6982176614/job/19000809120

[codebydant]

Hi @fnagel this could be silly, but can you change the regex pattern to use double quotes instead one?

pattern: "^.{0,72}(\n.*)*$"

and I think these parameters should be with double quotes

excludeTitle: "true"
excludeDescription: "true"
checkAllCommitMessages: "true"

[fnagel]

@dtcMLOps Haha, you never know ^^ but in this case, using double quotes did not do the trick: https://github.com/fnagel/t3extblog/actions/runs/6983519364/job/19004762599

Also tried to use double quotes for the three parameters but that didn't work out: https://github.com/fnagel/t3extblog/actions/runs/6983578961 https://github.com/fnagel/t3extblog/actions/runs/6983578666

Tried with my old regex too, without success.