-
Notifications
You must be signed in to change notification settings - Fork 54
Add automatic check for commit message format #36
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| { | ||
| "rules": { | ||
| "type-enum": [1, "always", [ | ||
| "Build", | ||
| "Ci", | ||
| "Docs", | ||
| "Feat", | ||
| "Fix", | ||
| "Perf", | ||
| "Refactor", | ||
| "Revert", | ||
| "Style", | ||
| "Test" | ||
| ]], | ||
| "header-full-stop": [2, "never"], | ||
| "header-trim": [2, "always"], | ||
| "subject-empty": [1, "never"], | ||
| "subject-full-stop": [2, "never", "."], | ||
| "subject-max-length": [1, "always", 72], | ||
| "subject-case": [2, "always", "sentence-case"], | ||
| "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:"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| name: Check Commit Message Format | ||
|
|
||
| on: [push, pull_request] | ||
|
|
||
| jobs: | ||
| commitmessage: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Commit message format | ||
| uses: wagoid/commitlint-github-action@v5 | ||
| with: | ||
| configFile: .commitlintrc.json |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,11 @@ To contribute your work you need to... | |
| 4. Push to the Branch (`git push origin newBranchName`) | ||
| 5. Open a Pull Request | ||
|
|
||
| ### Commit Message Format | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). |
||
|
|
||
| For detailed commit message guidelines, see the | ||
| [Commit Message Format](https://eclipse-openbsw.github.io/openbsw/sphinx_docs/doc/learning/commitmessage/index.rst). | ||
|
|
||
| ## Eclipse Contributor Agreement | ||
|
|
||
| Before your contribution can be accepted by the project team contributors must | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| Commit Message Format | ||
| ===================== | ||
|
|
||
| We have very precise rules over how our Git commit messages must be formatted. | ||
| This format leads to **easier to read commit history**. | ||
| Each commit message consists of a **header**, a **body**, and a **footer**. | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| <header> | ||
| <BLANK LINE> | ||
| <body> | ||
| <BLANK LINE> | ||
| <footer> | ||
|
|
||
| Commit Message Header | ||
| --------------------- | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| <type>: <subject line> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| │ │ | ||
| │ └─⫸ Subject in present tense. Capitalized. No period at the end. | ||
| │ | ||
| │ | ||
| └─⫸ Commit Type(optional): Build|Ci|Docs|Feat|Fix|Perf|Refactor|Test|Revert|Style | ||
|
|
||
| The ``<subject line>`` fields is mandatory. | ||
|
|
||
| Type | ||
| ++++ | ||
|
|
||
| You may optionally include a type at the start of the commit message. | ||
| You can choose one of the following commonly used types, or define your own if | ||
| it better describes the change: | ||
|
|
||
| - **Build**: Changes that affect the build system | ||
| - **Ci**: Changes to our CI configuration files and scripts | ||
| - **Docs**: Documentation changes | ||
| - **Feat**: A new feature | ||
| - **Fix**: A bug fix | ||
| - **Perf**: A code change that improves performance | ||
| - **Refactor**: A code change that neither fixes a bug nor adds a feature | ||
| - **Test**: Adding missing tests or correcting existing tests | ||
| - **Revert**: To undo previous commits | ||
| - **Style**: Changes related to formatting or whitespace adjustments | ||
|
|
||
| Subject Line | ||
| ------------ | ||
|
|
||
| Use the subject field to provide a brief description of the change: | ||
|
|
||
| - Use the imperative, present tense: *"change"* not *"changed"* nor *"changes"*. | ||
| - Stick to plain text in the subject. | ||
| - Limit the subject line to 72 characters. | ||
| - Capitalize the subject line. | ||
| - Do not end the subject line with a period (``.``). | ||
| - Avoid overly generic commit messages. | ||
|
|
||
| Commit Message Body | ||
| ------------------- | ||
|
|
||
| Explain the motivation for the change in the commit message body. This commit message should | ||
| explain **why** you are making the change. | ||
|
|
||
| You can include a comparison of the previous behavior with the new behavior in order to | ||
| illustrate the impact of the change. | ||
|
|
||
| It should also wrap at **72 characters per line**. | ||
| If the subject line is sufficiently descriptive, the body can be optional. | ||
|
|
||
| Commit Message Footer | ||
| --------------------- | ||
|
|
||
| The footer section is used for additional information that doesn't fit into the subject or body. | ||
| This can include references to GitHub issues and other PRs/discussions. | ||
|
|
||
| A common practice is to include a **Signed-off-by** line in the footer. | ||
| This line indicates who is responsible for the commit and is formatted as follows: | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| Signed-off-by: Full Name <email@example.com> | ||
|
|
||
| Good Example for a Commit Message | ||
| --------------------------------- | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| Docs: Create design documentation for main.cpp | ||
|
|
||
| - Add high-level overview of the main.cpp file | ||
| - Describe the purpose and functionality of each major function | ||
| - Include UML diagrams to illustrate class relationships | ||
|
|
||
| Resolves: #### | ||
| Signed-off-by: Full Name <email@example.com> | ||
|
|
||
| Revert Commits | ||
| -------------- | ||
|
|
||
| If the commit reverts a previous commit, it should begin with ``Revert:``, | ||
| followed by the header of the reverted commit. | ||
|
|
||
| The content of the commit message body should contain: | ||
|
|
||
| - Information about the SHA ID of the commit being reverted in the following format: | ||
|
|
||
| ``This reverts commit <SHA ID>`` | ||
|
|
||
| - A clear description of the reason for reverting the commit. | ||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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