Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: add github action for conventional commits #5732

Closed

Conversation

a-dubs
Copy link
Collaborator

@a-dubs a-dubs commented Sep 23, 2024

Proposed Commit Message

<type>(optional scope): <summary>  # no more than 72 characters

A description of what the change being made is and why it is being
made if the summary line is insufficient.  This should be wrapped at
72 characters.

If you need to write multiple paragraphs, feel free.

Fixes GH-NNNNN (GitHub Issue number. Remove line if irrelevant)
LP: #NNNNNN (Launchpad bug number. Remove line if irrelevant)

Additional Context

Test Steps

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@a-dubs a-dubs force-pushed the conventiona;-commits-github-action branch from ab8794b to 03ed079 Compare September 23, 2024 17:12
@a-dubs a-dubs force-pushed the conventiona;-commits-github-action branch from 03ed079 to 78322fb Compare September 23, 2024 17:14
@a-dubs
Copy link
Collaborator Author

a-dubs commented Sep 23, 2024

added the perf prefix that is supported by conventional commits but we opt not to support to test that our whitelist configuration works!

@a-dubs a-dubs marked this pull request as ready for review September 23, 2024 17:15
@a-dubs a-dubs marked this pull request as draft September 23, 2024 17:23
@a-dubs a-dubs force-pushed the conventiona;-commits-github-action branch from 95b206f to 78322fb Compare September 23, 2024 17:29
@a-dubs a-dubs marked this pull request as ready for review September 23, 2024 17:32
@a-dubs a-dubs closed this Sep 23, 2024
@TheRealFalcon
Copy link
Member

TheRealFalcon commented Sep 23, 2024

Given that we squash-merge by default and supply a separate proposed commit message, I can see this generating a lot of false positives. E.g., looking at my old userdata formats PR:

(cloud-init38) cow:cloud-init (fix-sysconfig-bridges) $ git lg userdata-formats 
* 22667de2c - (origin/userdata-formats, userdata-formats) comments (8 weeks ago) <James Falcon>
* b70e716bc - fix lint (8 weeks ago) <James Falcon>
* 884d1a717 - split the part handler stuff (8 weeks ago) <James Falcon>
* 0dcf106c0 - more comments (8 weeks ago) <James Falcon>
* 48533e9e4 - bring back section headers (8 weeks ago) <James Falcon>
* 15f4c1c7f - comments (8 weeks ago) <James Falcon>
* fc3c0563f - hopefully less confusing header/content-type (8 weeks ago) <James Falcon>
* 328505401 - Use bold rather than subheadings (8 weeks ago) <James Falcon>
* 7eacb07fb - update spelling list (8 weeks ago) <James Falcon>
* 9dd70e142 - docs: Overhaul user data formats documentation (8 weeks ago) <James Falcon>

I like adding new commits that I know will later be squashed as it's generally easier to re-review. For those commits, I generally don't put any thought into the commit messages.

For this to work, we either need to ignore the false positives if we know the main commit msg is correct, or we need to ask that every single commit have the conventional message even though we know it's going to get later squashed. I suppose that's not too much to ask to actually get proper commit messages on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants