renovate
renovate copied to clipboard
fix(commitMessage): adjust commit message rendering between flows
Changes
- extract commit message rendering into own class (analoguous to migration and onboarding)
- avoids creating the commit message prefix twice in comparison to the solution before
- matches commit message rendering between updates, migrations and onboarding
Context
Fixes https://github.com/renovatebot/renovate/issues/16839
Ongoing discussion https://github.com/renovatebot/renovate/discussions/16870
Documentation (please check one with an [x])
- [ ] I have updated the documentation, or
- [x] No documentation update is required
How I've tested my work (please tick one)
I have verified these changes via:
- [ ] Code inspection only, or
- [x] Newly added/modified unit tests, or
- [ ] No unit tests but ran on a real repository, or
- [ ] Both unit tests + ran on a real repository
It's difficult. The refactor would always involve using the CommitMessage classes for rendering and since they are rendered differently than the commitMessage template there would always be a functional change. This could be avoided by adding duplicated code into those classes and handling it differently between flows but I'm not sure if we get something out of this in comparison to my current change.
I would rather make it a breaking change and maybe even go a step further and also remove the deprecated commitMessage (and probably also the prTitle) option in this major to make sure that this change will not produce strange commit messages if people defined their own templates (also making it alot easier for future refactorings).
We would need at least one release where we strongly warn against using the commitMessage
in order to flush out any valid use cases we aren't currently aware of.
Then I see two options right now:
- Wait with this change until communication about commitMessage (and prTitle) is done
- Live with it breaking if someone uses a template that either manipulates/wraps the commitMessagePrefix or places the commitMessagePrefix in a different spot than in the beginning of the template.
Can you explain (2) in more detail or with examples? I'm not sure I understand
Of course.
Currently, the commit message is rendered via the template which is "{{{ commitMessagePrefix }}} {{{ someOtherStuff }}}" per default. In comparison, the CommitMessage class forces the prefix to always be in the front of the commit message (which is the sensible thing to do).
What we have to do right now, as we have to use a mix between the commitMessage template and the CommitMessage class until the template is removed, is to set CommitMessage.prefix = RenovateConfig.commitMessagePrefix, and CommitMessage.subject = RenovateConfig.commitMessage and set RenovateConfig.commitMessagePrefix = "" afterwards e.g.
RenovateConfig = { "commitMessage": "{{{ commitMessagePrefix }}} {{{ someOtherStuff }}}", "commitMessagePrefix": "", "someOtherStuff": "add stuff" }
CommitMessage = { "prefix": "issue-123:", "subject": "{{{ commitMessagePrefix }}} {{{ someOtherStuff }}}" }
Now when we render the template in the CommitMessage.subject with the RenovateConfig this will render correctly to "add stuff" as we removed the commitMessagePrefix from the config and finally yield the correct value "issue-123: add stuff" when using the CommitMessage.toString method.
Now assume the commitMessage template looks like this "tralala({{{ commitMessagePrefix }}}) {{{ someOtherStuff }}}", then rendering the CommitMessage.subject would result in "tralala() add stuff" and finally to "issue-123: tralala() add stuff".
Sorry for the layout, currently on my smartphone.
@JamieMagee @viceice what do you think about us explicitly removing custom commitMessage
support in v33? i.e. to enforce our structure? I think we've had a docs deprecation for a while, maybe we should add a warn in v32 and then remove in v33?
sounds good