We don't check commit messages, and even when looking at them in a Github PR, we might miss things.
For example when leaving out the blank line after the subject line, that's not visible in the Github UI.
But it is problematic when viewing the git log in a one-line mode (e.g. with
--oneline, or with a tool like
tig). Then the whole message is shown, not just the subject.
We could also check that the message starts with an uppercase letter, and optionally with something like
The last one seems better.
My first try showed that it might not support everything that perl regexes support, e.g.
\A did not work.
An alternative would be to implement it ourselves in a perl script, to be CI agnostic and to be able to call the check locally as well.
- AC 1: Run in OpenQA
- AC 2: A plan exists for the other repos
- Parent task set to #19720
This could be a good example of something that can be extended to other repos afterwards as well, e.g. os-autoinst-distri-opensuse, and such. As openQA can read the git log, e.g. for the "investigation" feature, having proper git commit messages makes sense and should be ensured.
#8 Updated by VANASTASIADIS 3 months ago
Open pr at https://github.com/os-autoinst/openQA/pull/4108.
- commit subject must begin either with a capital letter (A-Z) or with a tag ([A-Za-z0-9]:).
- commit subject must be no longer than 72 characters
- commit subject and commit body must be separated by at least one blank line
- each line in commit body must be no longer than 80 characters
you added to os-autoinst. Please reference the according PR. I suggest to follow https://github.com/os-autoinst/os-autoinst/pull/1751#issuecomment-905437822 and commit the changes to github.com/os-autoinst/os-autoinst-common/ to be reused in multiple repos.
#18 Updated by cdywan about 2 months ago
As mentioned in chat, I ran into a case where the check failed because of commits that didn't originate in my branch i.e.
- failed: "Merge pull request #4153 from cfconrad/allow_limit_parameter_for_job_overview. tinita suggested we can only check one, or all commits via the action. I wonder, can't the upstream action be fixed if this is a limitation of the action?
#21 Updated by cdywan about 1 month ago
Hit it again:
failed: "Merge pull request #4187 from os-autoinst/pitfalls_add_common_error_message, see https://github.com/kalikiana/openQA/runs/3569165279
#22 Updated by cdywan about 1 month ago
Open PR still needs to be merged: https://github.com/os-autoinst/os-autoinst-common/pull/12
It's not clear if AC2 is fulfilled, though. Do we have a plan for other repos?
#23 Updated by okurz about 1 month ago
merged https://github.com/os-autoinst/os-autoinst-common/pull/12 . I agree that AC2 is still not fixed.
#24 Updated by VANASTASIADIS about 1 month ago
I'm not sure I clearly understand AC2. Enabling this in other repos is as simple as copying the workflow file to the
.github/workflows dir of that repo. The file now exists in
common (but since it's a subrepo, it will have to be manually copied to each project's workflow dir).
Should I bump the due date a few days and try to figure out a more straightforward way, or resolve it?