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

conform.yaml: update to coreboot guidelines #614

Draft
wants to merge 1 commit into
base: dasharo
Choose a base branch
from

Conversation

macpijan
Copy link
Contributor

@macpijan macpijan commented Feb 7, 2025

Change first line of commit message from 80 to 75.

Set the commit body as required.

Source:
https://doc.coreboot.org/contributing/gerrit_guidelines.html#commit-message-guidelines

Change-Id: Iaab7a6a2c4adffa6b04b170b94cc98f3b157558e

Change first line of commit message from 80 to 75.

Set the commit body as required.

Source:
https://doc.coreboot.org/contributing/gerrit_guidelines.html#commit-message-guidelines

Change-Id: Iaab7a6a2c4adffa6b04b170b94cc98f3b157558e
Signed-off-by: Maciej Pijanowski <[email protected]>
@macpijan
Copy link
Contributor Author

macpijan commented Feb 7, 2025

Testing if it works as expected.

This conform tool should allow us to check if commit body exists.

For the Upstream-Status tag we would likely need a separate hook.

@@ -3,11 +3,11 @@ policies:
- type: commit
spec:
header:
length: 80
length: 75
imperative: false
invalidLastCharacters: .
body:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the length be specified separately for body? There is also a limit for it, and it used to be 72 (see e.g. https://review.coreboot.org/c/coreboot/+/82041/8..9//COMMIT_MSG#b12), I don't know if it changed recently or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@macpijan macpijan Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At present, the body is just checked if it exists. There is no enforcement on the line length whatsoever, it seems. So this would also need to be implemented.

@macpijan
Copy link
Contributor Author

macpijan commented Feb 7, 2025

Ok, the body check does not work for me, at least the way I would expect. Need to check.

EDIT:

OK I guess it's due to the Gerrit hooks adding: Change-Id: Iaab7a6a2c4adffa6b04b170b94cc98f3b157558e.

So we would need to modify this tool to add some more exceptions, preferably configurable via config file. Currently, it drops the DCO (Signed-off) line from this check only.

@macpijan macpijan marked this pull request as draft February 7, 2025 15:55
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