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

Add GH Action for commit message validation. #4167

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

OhmSpectator
Copy link
Member

@OhmSpectator OhmSpectator commented Aug 27, 2024

This PR introduces a new GitHub Actions workflow to automatically lint commit messages, ensuring they adhere to the recommended format. The workflow is triggered on pushes and pull requests to specific branches, including master and version branches. It includes a Python script that checks for the correct commit message format, ensuring that each message has a subject, an empty line separating the subject from the body, and a non-empty body. Additionally, the PR adds a requirements.txt file for managing dependencies and updates the .gitignore file to exclude the virtual environment directory. This enhancement helps maintain consistent and readable commit messages across the repository.

@OhmSpectator OhmSpectator force-pushed the feature/commitlint branch 8 times, most recently from cfe9550 to 2c5cbfe Compare August 27, 2024 00:49
@OhmSpectator OhmSpectator changed the title github: Add Commitlint GH Action for commit message validation. [WIP] Add Commitlint GH Action for commit message validation. Aug 27, 2024
@OhmSpectator OhmSpectator force-pushed the feature/commitlint branch 3 times, most recently from 265bc19 to eb51f66 Compare August 27, 2024 01:09
Copy link

codecov bot commented Aug 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.69%. Comparing base (695652b) to head (eb51f66).
Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4167   +/-   ##
=======================================
  Coverage   19.69%   19.69%           
=======================================
  Files           8        8           
  Lines        2600     2600           
=======================================
  Hits          512      512           
  Misses       1985     1985           
  Partials      103      103           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@OhmSpectator OhmSpectator changed the title [WIP] Add Commitlint GH Action for commit message validation. Add GH Action for commit message validation. Aug 27, 2024
@OhmSpectator OhmSpectator marked this pull request as ready for review August 27, 2024 02:19
@OhmSpectator OhmSpectator self-assigned this Aug 27, 2024
recommended format. The recommended format is as follows:
* Commit message should have a subject and body
* An empty line should separate the subject and body
* The body should not be empty (it should contain more than just "Signed-off-by")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also impose line length limit?

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 thought about it, but we have too many people for whom even these rules can be too strict =D So, let's introduce the restrictions step by step.

Copy link
Contributor

@milan-zededa milan-zededa left a comment

Choose a reason for hiding this comment

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

Thanks for both new linters, you had a very productive night it seems :)

repo = git.Repo(search_parent_directories=True)

# Fetch the latest from origin/master to ensure we are comparing correctly
repo.remotes.origin.fetch('master')
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
Member Author

Choose a reason for hiding this comment

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

Yep, will have to fix it.

# Fetch the latest from origin/master to ensure we are comparing correctly
repo.remotes.origin.fetch('master')

commits = list(repo.iter_commits('origin/master..HEAD'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work with PRs into stable branches?

Copy link
Member Author

Choose a reason for hiding this comment

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

Most probably not. I will switch to the GH variable.

@OhmSpectator OhmSpectator force-pushed the feature/commitlint branch 2 times, most recently from 8ffd216 to 127d7c7 Compare August 27, 2024 09:35
@rene
Copy link
Contributor

rene commented Aug 27, 2024

Thanks for this PR @OhmSpectator , I think we were missing for a long time this feature, definitely it will help (enforce 😄 ) us to standardize and improve our commit messages.... I have just one comment, it could also check for a capitalized first word of the body message, as asked in our guidelines: https://github.com/lf-edge/eve/blob/master/CONTRIBUTING.md#commit-messages , but it can be done in a further iteration as well...

@OhmSpectator
Copy link
Member Author

I have just one comment, it could also check for a capitalized first word of the body message, as asked in our guidelines: https://github.com/lf-edge/eve/blob/master/CONTRIBUTING.md#commit-messages ,

Lol, I have never seen these guidelines =D
Will add a check now.

@OhmSpectator
Copy link
Member Author

@rene, I've added the capital letter check.

Introduced a GitHub Actions workflow (`commit-messages.yml`) to lint
commit messages on `master`, version branches, and stable branches.

Added `check_commit_messages.py` script to validate commit messages
against recommended format (subject, empty line, body).

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
@rene
Copy link
Contributor

rene commented Aug 28, 2024

Thanks again @OhmSpectator , I'm approving this PR but I would like to drop one more idea here: it would be really nice to integrate this check as a git hook, you can provide a custom script to check commit message in .git/hooks/commit-msg, maybe for a future PR....

@OhmSpectator
Copy link
Member Author

Yeah, let's add a hool the next step. Let the team first get used to the rules by seeing failures in the GH Actions =)

@OhmSpectator OhmSpectator merged commit 3900c2d into lf-edge:master Aug 28, 2024
36 checks passed
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.

4 participants