ci: add commit checks for PRs

[Mirko-von-Leipzig]

This PR introduces a new github workflow which ensures all commits in a PR conform to our requirements. It checks for the following:

1. conventional commit messages
2. used git commit --signoff
3. no extra merge commits

Conventional commits and --signoff

The first two points are taken care of by the NPM commitlint package. Unfortunately, they provide guides for CircleCI and TravisCI but not for Github Actions (open issue). I managed to get it working after some trial and error.

There is a 3rd party Github Action which looks great, but unfortunately I ran into this bug. It also seemed like a lot of code to audit, considering we don't need much.

Another alternative could be to use git interpret-trailers to check for --signoff, but since this came bundled with commitlint I didn't explore further.

no extra merge commits

This gets checked with some simple bash code walking the PR commits using git log --merges --ancestry-path. Luckily the Github PR event contains the HEAD and number of commits in the PR.

Why a new workflow file?

It has different run on requirements and therefore requires its own file (I think?).

It made sense to me that it should only run on actual PRs. It would otherwise trigger (and probably fail) when wanting to test things quickly.

I also enabled it for PRs into any branch (and not just into master), since its likely the code would eventually make its way into a PR for master, but by that time it may be too late to get the commit requirements fixed.

cargo audit

I've also added in the ignore for the two hyper RUSTSECs.

[koivunej]

Could you add the missing newline in a non-signed off commit, possibly being result of something you merged into this PR branch, later rebase it?

[Mirko-von-Leipzig]

Could you add the missing newline in a non-signed off commit, possibly being result of something you merged into this PR branch, later rebase it?

I'll add a dummy commit 👍 -- done. Should fail CI now (I hope)

[koivunej]

Oki could you now add a merge commit as well?

[Mirko-von-Leipzig]

Oki could you now add a merge commit as well?

mmm. What I did broke something here. It seems the commitlint check is walking the wrong commits now. So the wrong step fails.

--- update ---
@koivunej

Turns out HEAD~N doesn't work once there's a merge commit in the mix. Which makes sense. Luckily GH provides the base branches HEAD commit as well. So now the commit range is calculated (and shared between steps using env variables) using:

echo "PR_HEAD=${{ github.event.pull_request.head.sha }}" >> $GITHUB_ENV
echo "PR_BASE=${{ github.event.pull_request.base.sha }}" >> $GITHUB_ENV

Since the code has changed I'll start the "tests" from the start again.

• base PR
• non-signed commit
• with merge commit
• non-conventional-commit

non-signed commit

⧗   input: feat: unsigned
✖   message must be signed off [signed-off-by]

with merge commit

PR contains merge commits:
commit f3489557b0808e1ee5b078f10dfa9afa10c8a2e3 Merge: 8da2d21 dec3383 Author: Mirko von Leipzig <mirko.vonleipzig@gmail.com> Date: Wed Aug 11 11:10:55 2021 +0200 Merge branch 'merge_branch' into gh_action_checks

non-conventional-commit (was also unsigned)

⧗   input: unconventional
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]
✖   message must be signed off [signed-off-by]

[koivunej]

This is looking good. I think the non-action usage of javascript is ok without auditing, even though the thing has a long lockfile..

Could you give docs/CONTRIBUTING.md a once over for for example listing what checks are now done automatically and so on?

Otherwise this looks good to start the 24h merge period.

[koivunej]

Thank you @Mirko-von-Leipzig very much!