Skip to content

Add automatic check for commit message format#36

Closed
shamitha-shashidhara wants to merge 1 commit intoeclipse-openbsw:mainfrom
esrlabs:formatcheck
Closed

Add automatic check for commit message format#36
shamitha-shashidhara wants to merge 1 commit intoeclipse-openbsw:mainfrom
esrlabs:formatcheck

Conversation

@shamitha-shashidhara
Copy link
Contributor

@shamitha-shashidhara shamitha-shashidhara commented Dec 6, 2024

  • Add GitHub Actions workflow to commit messages on push and pull request events.
  • Create custom configuration file (.commitlintrc.json) to enforce commit message standards.
  • Document commit message format in CONTRIBUTING.md

@shamitha-shashidhara shamitha-shashidhara marked this pull request as draft December 24, 2024 09:48
@shamitha-shashidhara shamitha-shashidhara force-pushed the formatcheck branch 3 times, most recently from 26f2941 to f384393 Compare December 30, 2024 05:12
@shamitha-shashidhara shamitha-shashidhara marked this pull request as ready for review January 8, 2025 05:14
"header-trim": [2, "always"],
"subject-empty": [1, "never"],
"subject-full-stop": [2, "never", "."],
"subject-max-length": [1, "always", 50],
Copy link
Contributor

Choose a reason for hiding this comment

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

50 chars will lead to cryptic subject lines, I strongly vote for 72 chars like in the body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated it to 72 chars.

"body-leading-blank": [2, "always"],
"body-max-line-length": [1, "always", 72],
"body-case": [1, "always", "sentence-case"],
"signed-off-by": [1, "always", "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.

I don't like this very much. Let's discuss this in the public OpenBSW round.

"revert",
"style",
"test"
]],
Copy link
Contributor

Choose a reason for hiding this comment

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

The terms are overlapping, e.g. what to use when refactor the code which improves performance? What if you fix the doc? What would you do when working on process descriptions, is that doc? What would you choose when removing a function? Is that refactor?

Personally, I would not limit this to a certain set of term, but recommend some terms, e.g. start of "Revert" if you revert a comment.

Btw, starting a commit message with lower case looks strange to my eyes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tool is designed to include a type in the commit message header. However, based on your suggestion, users are free to use any custom type; this will only generate a warning, not an error. Similarly, if a commit message does not include a type at all, it will still generate a warning, not an error.

I have also updated the commit message to start with an uppercase letter.

Additionally, we can remove the type restriction from the JSON configuration file to allow contributors more flexibility but the tool will continue to function as designed, issuing a warning if the type is missing, not error

4. Push to the Branch (`git push origin newBranchName`)
5. Open a Pull Request

### Commit Message Format
Copy link
Contributor

Choose a reason for hiding this comment

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

Imho this should be moved to the Sphinx guideline (GitHub pages) and only link from here to the guideline to avoid building up a "second source".

CONTRIBUTION should only summarize what is needed to contribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The detailed commit message guidelines have been moved to the Sphinx documentation (GitHub Pages).
The CONTRIBUTING.md now contains a link to the full guidelines in the documentation.

CONTRIBUTING.md Outdated
* **revert**: To undo previous commits
* **style**: Changes related to formatting or whitespace adjustments

##### Subject line
Copy link
Contributor

Choose a reason for hiding this comment

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

Use always "Title Case": https://apastyle.apa.org/style-grammar-guidelines/capitalization/title-case

Done for several other headings already in this file.

Copy link
Contributor Author

@shamitha-shashidhara shamitha-shashidhara Oct 3, 2025

Choose a reason for hiding this comment

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

Acknowledged

CONTRIBUTING.md Outdated

##### Subject line

Use the subject field to provide a breif description of the change:
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: breif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged

- Add GitHub Actions workflow to lint commit messages
on push and pull request events.
- Create custom commitlint configuration file (.commitlintrc.json)
to enforce commit message standards.
- Document commit message format in CONTRIBUTING.md

Signed-off-by: Shamitha Shashidhara <shamitha.shashidhara@accenture.com>
@johannes-esr
Copy link
Contributor


.. code-block:: text

<type>: <subject line>
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't want any additional metadata like in the subject line. So this particular check would need to be reversed.

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