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.

Sign up for GitHub

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

Jump to bottom

[RFC]: Adopt Conventional Commits #2264

Open
DominicLuidold opened this issue Apr 12, 2024 · 1 comment · May be fixed by #2273
Open

[RFC]: Adopt Conventional Commits #2264

DominicLuidold opened this issue Apr 12, 2024 · 1 comment · May be fixed by #2273

Comments

@DominicLuidold
Copy link
Contributor

DominicLuidold commented Apr 12, 2024 •
edited

The goal of this RFC is to determine whether adopting the Conventional Commits specification for commits targeting the master branch (or any other branch) is feasible for the NelmioApiDocBundle maintainer community. This is a continuation of a (very brief) discussion as part of #2261 which touched the aspect of commit messages.

Scope

As this repository's workflow is based on opening pull requests for changes targeting a specific branch (with very few direct pushes to the master branch, for example) the Conventional Commits specification will only have to be adopted for pull request titles, not individual commits within a pull request.1

Main benefits

  • Communicating the nature of changes to teammates, the public, and other stakeholders.
  • Making it easier for people to contribute to your projects, by allowing them to explore a more structured commit history.

See the following mockup of currently open pull requests for this repository compared to the current state, showcasing the quoted2 benefits:

Conventional-Commits_NelmioApiDocBundle_Mockup

Actual PR titles

Conventional-Commits_NelmioApiDocBundle_Current

  • Automatically generating CHANGELOGs.

This could potentially also be seen as a benefit, though implementing an automated changelog and release notes is out of scope of this RFC.

Implementation

Enforcing the Conventional Commits specification for pull request titles, while keeping the maintainer/contributor DX in mind, is achievable using a combination of two GitHub actions:

A working proof of concept implementation can be found here: DominicLuidold@dd762d4.

The following sample pull request showcases the validation/linting comment automatically created when an invalid title is presented: DominicLuidold#1. Once the title has been correctly updated, the comment will be automatically removed.

Tasks for successful implementation & usage of Conventional Commits

  • New GitHub Actions workflow linting GitHub pull request titles
  • Allow squash merging1 as only enabled merge option for pull requests3
  • Adapted commit message prefix4 for Dependabot pull requests
  • No direct commits to any branch without a pull request
  • Updated contributing guidelines

Open discussion points

While the Conventional Commits specification is clear how a PR/commit should be written, the following aspects can be changed on a per project basis:

  • The available list of types is currently based on https://github.com/commitizen/conventional-commit-types but can be freely configured
  • The available list of scopes is currently unrestricted, could be configured to specifically enforcing mentioning one (ore more) GitHub issues with a #1234 syntax or any other string pattern that can be checked by a regex
  • Casing of the subject, e.g. has to start with an uppercase or lowercase character

Footnotes

  1. Based on the assumption that Allow squash merging is the only enabled merge option for pull requests, see Configuring commit squashing for pull requests ↩ ↩2

  2. https://www.conventionalcommits.org/en/v1.0.0/#why-use-conventional-commits ↩

  3. While changing the approach, it may be beneficial to switch to using pull request title and description as default to use the PR's description as commit body rather than a list of commits that can be reviewed at any time looking at the merged PR. This approach is, for example, also used by Symfony itself. ↩

  4. https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#commit-message ↩
@DjordyKoert
Copy link
Collaborator

Looking really good and I am really in favor of adding this. 😄 🎉

sticky-pull-request-comment

Adding a comment does not feel necessary in my opinion. The failing action-semantic-pull-request action in combination with a proper contributing guideline should be enough information to help guide a contributor to fixing their PR.

The available list of types is currently based on commitizen/conventional-commit-types but can be freely configured

I don't see a reason to differ from the default. :)

The available list of scopes is currently unrestricted, could be configured to specifically enforcing mentioning one (ore more) GitHub issues with a #1234 syntax or any other string pattern that can be checked by a regex

I don't think we have to enforce creating an issue before creating a PR at this current time.

Casing of the subject, e.g. has to start with an uppercase or lowercase character

Let's use lowercase (the default)

@DominicLuidold DominicLuidold linked a pull request Apr 18, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore(#2264): adopt Conventional Commits DominicLuidold/NelmioApiDocBundle
2 participants
@DominicLuidold @DjordyKoert