11

I have a Github repository, installed commitlint and husky locally and would like to setup a workflow running commitlint on every commit of a push when validating pull requests. On the main branch older commits are not following the conventional commit rules.

I created a separate branch, based on this comment

https://github.com/conventional-changelog/commitlint/issues/586#issuecomment-657226800

I started with this workflow

name: Run commitlint on pull request

on: pull_request

jobs:
  run-commitlint-on-pull-request:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 0

      - name: Setup Node
        uses: actions/setup-node@v2
        with:
          node-version: 14.x

      - name: Install dependencies
        run: npm install

      - name: Validate all commits from PR
        run: npx commitlint --from HEAD~${{ github.event.pull_request.commits }} --to HEAD --verbose

I made two more commits following the conventional commit rules and started a pull request

  • I expected the workflow wouldn't run because I doesn't exist on the main branch yet.
    • Actually it runs
  • I exptected the workflow to check PR commits only
    • The workflow fails because it starts validating EVERY commit in the main branch. And since I know older commits don't follow the rules, this will never pass.

The first solution coming to my mind would be to rebase everything and rename each commit to follow the rules but this would require a huge effort.

I'm not sure if I have to improve this line here

npx commitlint --from HEAD~${{ github.event.pull_request.commits }} --to HEAD --verbose

to check commits from the PR only (unfortunately I don't know what needs to get fixed there).

Do you have any ideas or is rebasing and renaming the only solution?

Question3r
  • 2,166
  • 19
  • 100
  • 200
  • Try `npx commitlint --from $commit --to HEAD --verbose || exit 1` – phd Apr 28 '21 at 19:55
  • sorry, unfortunately the `|| exit 1` didn't help. the workflow still passes – Question3r Apr 28 '21 at 20:11
  • Does `npx commitlint` exit with error codes at all? – phd Apr 28 '21 at 20:36
  • sorry, I wasn't able to find out. But the image shows I have to fix the syntax I think? – Question3r Apr 29 '21 at 05:19
  • IMO if you want to test commits one by one `--from $commit --to HEAD` is wrong, it should be one commit, something like `--from $commit~ --to $commit`. Or instead of the loop test all commits at once: `--from ${{ github.base_ref }} --to ${{ github.head_ref }}` without a loop. – phd Apr 29 '21 at 07:43
  • I tested both suggested solutions. The first one didn't work, the error remains, the workflow passes. I will update my question to show the result of your second solution – Question3r Apr 29 '21 at 07:57
  • Move whatever function you want to run into a shell script that exits with non-zero on failure, test it locally, then run the same script in the workflow. This approach will shorten your debugging cycle by an order of magnitude. – DannyB Apr 29 '21 at 08:42
  • yeah the problem is that I can't test this `npx commitlint --from ${{ github.base_ref }} --to ${{ github.head_ref }} --verbose` outside from Github – Question3r Apr 29 '21 at 08:45
  • `git rev-list` [documentation](https://git-scm.com/docs/git-rev-list) suggests to use the three dots `...` notation to list merges. This seems more in line with the purpose or alternatively `git rev-list main test --not $(git merge-base --all main test)` – Constantin Konstantinidis May 01 '21 at 06:19
  • @ConstantinKonstantinidis I moved from two dots to three dots as suggested. Now the error from the first image was thrown and the PR passes (shoudln't pass). I also tried your second approach and get the same result. Is this the approach you wanted to try? `for commit in $(git rev-list ${{ github.base_ref }} ${{ github.head_ref }} --not $(git merge-base --all ${{ github.base_ref }} ${{ github.head_ref }} )); do` – Question3r May 01 '21 at 18:10

3 Answers3

11

Solution

The straight-forward solution is to use the --to and --from arguments of commitlint with the SHA-1 values instead of the branch names or relative references. On the one hand, this reliably solves the problem of unknown revisions or paths in the working tree. On the other hand, only commits in scope of the PR will be checked. As a sidenote: GitHub uses the same references (SHA-1) for the ad-hoc merge that is being checked-out.

We need the base-SHA as well as the head-SHA. In a GitHub action those values are available in the pull-request object of the event in the github-context.

Therefore, you can use the following line which is tested and works as expected:

npx commitlint --from ${{ github.event.pull_request.base.sha }} --to ${{ github.event.pull_request.head.sha }} --verbose

Demo

Here is a POC repository on GitHub with 3 test cases (pull-requests).

Complete workflow

name: Run Commitlint on PR

on:
  pull_request:

jobs:

  run-commitlint-on-pr:
    runs-on: ubuntu-latest

    steps:

      - uses: actions/checkout@v2
        with:
          fetch-depth: 0

      - name: Setup Node
        uses: actions/setup-node@v2
        with:
          node-version: 14.x

      - name: Install dependencies
        run: npm install

      - name: Validate all commits from PR
        run: npx commitlint --from ${{ github.event.pull_request.base.sha }} --to ${{ github.event.pull_request.head.sha }} --verbose
Matt
  • 12,848
  • 2
  • 31
  • 53
  • What about commits to pushed directly to master without a pull request? – Kutsan Kaplan Jun 08 '21 at 18:12
  • That was actually poor of me to say since commits pushed to master shouldn't actually be overwritten. Sorry for asking. – Kutsan Kaplan Jun 08 '21 at 18:19
  • @KutsanKaplan Your question has merit. You need to specify the branches in the `on:` section. Have a look at the [official documentation](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#on). – Matt Jun 08 '21 at 18:27
3

git rev-list is considered because the commit of the pull request (PR) seems invalid. No loop should be required.

This issue hints to checkout the PR branch which seems simpler than fetching the PR commits. From the question, testing on default branch does not seem required.

- uses: actions/checkout@v2
        with:
          fetch-depth: 0
          ref: ${{ github.event.pull_request.base.sha }}

The lint instruction would be:

npx commitlint --from ${{ github.event.pull_request.base.sha }} --verbose

Documentation of pull-request payload does not offer the list of commits right away.

Constantin Konstantinidis
  • 1,307
  • 12
  • 15
  • hey, thanks for your help! :) I'm going to try out your suggestion with this workflow https://pastebin.com/zT8X8wLw soon – Question3r May 02 '21 at 05:59
  • So I tried it out but unfortunately the lint instruction doesn't work. I uploaded the full log on imgur https://imgur.com/a/XFvYnK5 the short error message is: `[input] is required: supply via stdin, or --env or --edit or --from and --to` – Question3r May 02 '21 at 07:32
  • Indeed, stdin is used when no parameter is provided. Suggested command is supplemented with --from flag. – Constantin Konstantinidis May 02 '21 at 08:30
  • Unfortunately that didn't help. The workflow passes although I created one valid commit message, one invalid and after that I created a valid last one. I would expect that the workflow fails because the second one is invalid. But the workflow passes. – Question3r May 02 '21 at 08:36
  • You seem to imply that only the relevant commits are tested. The linting rules are not in the question. What is the expected discrepancy ? – Constantin Konstantinidis May 02 '21 at 09:04
  • I expect that if I create a pull request with 2 valid commits and 1 invalid commit, the process will fail and show me which commits are invalid. If I create a pull request with only valid commits, it should succeed. If I create a pull request with invalid commits only, the process should fail and show me information. – Question3r May 02 '21 at 10:42
  • That is understood from above. So, linting fails on one commit but the github-action does not. Could you post the log to check details ? – Constantin Konstantinidis May 02 '21 at 12:06
  • does this help? https://pastebin.com/MTW6KXBH I will describe what I did for reproduction in the next comment – Question3r May 02 '21 at 13:40
  • I setup npm, commitlint and husky as described here https://commitlint.js.org/#/guides-local-setup. I create the workflow file. Now I move into another branch and modify the README and make some commits. After that I can create PRs from this branch and test the functionality – Question3r May 02 '21 at 13:41
  • The commitlint instruction does not seem to do much as the log is empty. git checkout occurs but the object is the head of the PR and not the base of it. There is a git log -1 instruction. It could be useful to set 4 to check that indeed all commits are available using the existing ref. – Constantin Konstantinidis May 02 '21 at 14:13
  • you mean like adding an additional step e.g. `run: git log 4` ? – Question3r May 02 '21 at 19:00
3

I used the following github action: https://github.com/wagoid/commitlint-github-action

From the README of the linked repository:

Create a github workflow in the .github folder, e.g. .github/workflows/commitlint.yml:

name: Lint Commit Messages
on: [pull_request]

jobs:
  commitlint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
        with:
          fetch-depth: 0
      - uses: wagoid/commitlint-github-action@v5
Julian Espinel
  • 2,586
  • 5
  • 26
  • 20