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

Try to validate commit headers. #6630

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbien
Copy link
Member

@mbien mbien commented Oct 28, 2023

Back when I setup the paperwork job, I didn't know that gh can generate patch files yet. The problem was to figure out what commits are actually new in the PR. I gave it a few attempts but got distracted with more important things.

this adds:

  • simple script to check if all commits have proper email addresses
  • looks for empty or single word subject messages
  • warns about missing blank lines after subject
  • logs errors/warnings to the github actions summary page

output in workflow summary:

image

output in job log:

image

@mbien mbien added CI continuous integration changes ci:no-build [ci] disable CI pipeline labels Oct 28, 2023
@mbien mbien requested a review from ebarboni October 28, 2023 00:29
@mbien mbien force-pushed the ci-email-checker branch 2 times, most recently from 0770006 to 127107a Compare October 28, 2023 20:35
@mbien mbien force-pushed the ci-email-checker branch 4 times, most recently from d17b7ab to a3f84da Compare December 15, 2023 16:18
@ebarboni
Copy link
Contributor

well late :D will need rebase.

@mbien
Copy link
Member Author

mbien commented Sep 26, 2024

rebased, removed string templates and fixed a bug

@mbien mbien removed the ci:no-build [ci] disable CI pipeline label Sep 26, 2024
java: [ '17' ]
java: [ '23' ]
Copy link
Member Author

Choose a reason for hiding this comment

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

I hope this doesn't cause issues, but we will see next time this PR fully builds and all paperwork checks run, I removed the no-build label now.

Copy link
Member Author

Choose a reason for hiding this comment

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

this seems to work. But I might change it back anyway. Probably not worth the risk to run sig test-tests on JDK 23.

@ebarboni
Copy link
Contributor

LGTM

 - simple script to check if all commits have proper email addresses
 - looks for empty or single word subject messages
 - warns about missing blank line after subject
 - logs errors/warnings to the github actions summary page
@mbien
Copy link
Member Author

mbien commented Sep 27, 2024

removed the test commits and updated the error messages a bit

@mbien mbien marked this pull request as ready for review September 27, 2024 20:58
Comment on lines +108 to +114
boolean checkBlankLineAfterSubject(String blank, int i) {
if (!blank.isBlank()) {
println("::warning::blank line after subject recommended in commit " + i + " (subject over 50 char limit?)");
// return false;
}
return true;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

i am not sure about this. Will anyone even look at the warnings? Should I turn this off for now?

I suppose a better place for checks like this would be the NB commit window ;)

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

Successfully merging this pull request may close these issues.

2 participants