Merge ~sbeattie/ubuntu-cve-tracker/+git/ubuntu-cve-tracker:git-commit-hooks-pre-merge_hook into ubuntu-cve-tracker:master
Status: | Needs review |
---|---|
Proposed branch: | ~sbeattie/ubuntu-cve-tracker/+git/ubuntu-cve-tracker:git-commit-hooks-pre-merge_hook |
Merge into: | ubuntu-cve-tracker:master |
Diff against target: |
91 lines (+15/-53) 3 files modified
Makefile (+2/-2) dev/null (+0/-51) scripts/git-hooks/pre-merge-commit (+13/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Eduardo Barretto | Needs Information | ||
Alex Murray | Approve | ||
Review via email: mp+457285@code.launchpad.net |
Commit message
git-hooks: use pre-merge-commit over prepare-commit-msg
Replace/remove the prepare-commit-msg with a pre-merge-commit hook that
invokes the pre-commit infrastructure we already have.
When we originally started using git hooks to perform check-syntax
against UCT, the prepare-commit-msg hook was added (in 2019) because
git's pre-commit hook does not get performed on git merges, and there
were times when we would want to merge branches from people who do
not have enough of a setup for check-syntax to work.
However, git has had a pre-merge-commit hook since git 2.24 [1], and
present since at least Ubuntu 20.04 LTS, so we should make use of that.
Additionally, the prepare-commit-msg hook attempted to duplicate the
pre-commit hook but was never kept in sync, so choose here to use
the default upstream pre-merge-commit hook, which just invokes the
pre-commit hook.
Description of the change
Thinking about git commit hooks from Amir's merge request reminded me that we were using the prepare-commit-msg hook to work around git's failure to use the pre-commit hook for merges. Git 2.24 added support for a pre-merge-commit hook, so this merge proposal converts UCT to use that, and set it up via 'make dev-setup'.
One thing this change does NOT do, however, is remove an existing prepare-commit-msg hook if it exists (which it should, because make dev-setup would install it into place).
Unmerged commits
- 2963895... by Steve Beattie
-
unit-tests:0 (build) check-cves:0 (build) 1 → 2 of 2 results First • Previous • Next • Last
LGTM except I wonder if we should make this get rid of the old prepare-commit-msg hook as well to keep things cleaner.