-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
ci: Enforce conventional commit format in PR title #398
Conversation
df74d1b
to
633d763
Compare
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.
My question is: since we impose to squash commits before merging, the user has a dialog box to define the squash-commit message. He can therefore easily override this PR title rule. Is this really relevant?
I welcome the initiative, but IMHO, i think it's premature and should be done at the commits level.
I love this! Can we mention expected rules in the contributing guide? |
This is a good point; I will do some research to see how to deal with this. In the meantime, only people with write access to the repo (i.e. the Probabl product team) can do this when merging a PR, so as long we're minimally disciplined I don't think that's an issue |
You mean check individual commits inside a PR? I don't mind, it just heavily increases dev friction and in the end all the commits are squashed |
This is a pre-requisite for proper release management, e.g. for updating the changelog automatically. It's a simple change, and it will immediately make our main branch cleaner. |
Time for joke: if we're disciplined, we don't need such a test, since only the maintainers can merge a PR and therefore edit the merge-commit message and respect the convention :) . I'm truly convinced that if we want to base an automatic release management on conventional commits, we need to add some strong rules that can't be ignored. To err is human. I don't know if its possible to run a workflow after squashing but before merging. |
I agree with @thomass-dev. We can start by describing good conventions, exercise them manually and then implement them automatically.
I am not sure. I don't like changelogs based solely on automatically generated statements; they should primarily have an editorial perspective. |
Obviously, you need much less discipline to not change a green PR message, than to make sure every PR you review abides by a full convention. This is a net positive.
I don't think this convention will be strictly followed unless there are automatic checks for it; furthermore, I don't want to do a job that this PR could easily achieve.
Point taken; taking the list of commits is a first step, that can then be used to write up more digestible updates. |
e981d25
to
4105294
Compare
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.
Please add a section on conventional commits in the contributing guide, mentioning what they are and what we expect.
842e9a4
to
6199a02
Compare
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.
I second Thomas' request for changes, otherwise all good. Thank you!
3769cac
to
823128c
Compare
858bca9
to
1fac371
Compare
1fac371
to
1d1fd14
Compare
1d1fd14
to
fd85154
Compare
dd29452
to
149c2b3
Compare
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.
Nice work! I like it.
Closes #66