Project

General

Profile

Actions

action #95581

closed

coordination #39719: [saga][epic] Detection of "known failures" for stable tests, easy test results review and easy tracking of known issues

coordination #19720: [epic] Simplify investigation of job failures

ci: Use a git commit message style checker size:S

Added by tinita almost 3 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
Feature requests
Target version:
Start date:
2021-07-16
Due date:
% Done:

0%

Estimated time:

Description

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 t:, ci: etc.

Possible candidates:

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.

Acceptance Criteria

  • AC 1: Run in OpenQA
  • AC 2: A plan exists for the other repos

Related issues 2 (2 open0 closed)

Related to openQA Project - action #97454: Decide and implement predefined tags for the commit message checkerNew2021-08-24

Actions
Copied to openQA Project - action #99066: ci: Use a git commit message style checker in more reposNew

Actions
Actions #1

Updated by tinita almost 3 years ago

  • Status changed from New to Workable
  • Target version changed from future to Ready
Actions #2

Updated by tinita almost 3 years ago

  • Status changed from Workable to New
Actions #3

Updated by okurz almost 3 years ago

  • 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.

Actions #4

Updated by ilausuch over 2 years ago

  • Subject changed from ci: Use a git commit message style checker to ci: Use a git commit message style checker size:S
Actions #5

Updated by ilausuch over 2 years ago

  • Description updated (diff)
  • Status changed from New to Workable
Actions #6

Updated by VANASTASIADIS over 2 years ago

  • Assignee set to VANASTASIADIS
Actions #8

Updated by VANASTASIADIS over 2 years ago

Open pr at https://github.com/os-autoinst/openQA/pull/4108.

Checks:

  • 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
Actions #9

Updated by tinita over 2 years ago

  • Status changed from Workable to Resolved

The PR was already merged.

Actions #10

Updated by tinita over 2 years ago

  • Status changed from Resolved to Workable

Oh wait, AC 2 is still missing.

Actions #11

Updated by VANASTASIADIS over 2 years ago

  • Related to action #97454: Decide and implement predefined tags for the commit message checker added
Actions #12

Updated by okurz over 2 years ago

As next repos to add this to I suggest all repos within https://github.com/os-autoinst/

Actions #13

Updated by okurz over 2 years ago

  • Status changed from Workable to In Progress
Actions #14

Updated by okurz over 2 years ago

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.

Actions #15

Updated by livdywan over 2 years ago

@VANASTASIADIS Is this still in progress?

Actions #16

Updated by VANASTASIADIS over 2 years ago

It remains to follow @okurz's advice in the last comment and this is ready to be resolved.

Actions #17

Updated by livdywan over 2 years ago

VANASTASIADIS wrote:

It remains to follow @okurz's advice in the last comment and this is ready to be resolved.

Can you please clarify what that means? Private chat doesn't count, we need it stated here and get the ticket resolved ;-)

Actions #18

Updated by livdywan over 2 years 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?

Actions #19

Updated by tinita over 2 years ago

I think next time this happens we should try to find out why the action included those commits.

Actions #20

Updated by VANASTASIADIS over 2 years ago

  • Due date set to 2021-09-24
  • Status changed from In Progress to Feedback

Leaving this ticket open to investigate possible errors in checking older commit messages. Setting the due-date accordingly.

Actions #21

Updated by livdywan over 2 years 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

Actions #22

Updated by livdywan over 2 years 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?

Actions #23

Updated by okurz over 2 years ago

merged https://github.com/os-autoinst/os-autoinst-common/pull/12 . I agree that AC2 is still not fixed.

Actions #24

Updated by VANASTASIADIS over 2 years 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?

Actions #25

Updated by okurz over 2 years ago

  • Copied to action #99066: ci: Use a git commit message style checker in more repos added
Actions #26

Updated by okurz over 2 years ago

  • Status changed from Feedback to Resolved

I created #99066 which effectively is what we need to cover AC2 so no need to continue further here, resolving. Thanks!

Actions #27

Updated by okurz over 2 years ago

  • Due date deleted (2021-09-24)
Actions

Also available in: Atom PDF