Skip to content

Refactor commit message template evaluation

What does this MR do and why?

This refactors merge commit message template generation introduced in !64437 (merged). It introduces no changes visible to user.

Before:

  • It was evaluating every variable to check whether it is empty - even if the variable was not used in template.
  • If the variable was used in template, and wasn't empty, it would evaluate its value twice.
  • Variable value could not be nil, as it would cause the placeholder that is in the middle of the line to be left in place in a message.

The problem was discussed in !75819 (comment 754368963) - it made the specs harder to understand, as we had to mock properties that should not be required in a given specs.

The downside is that I've removed the use of StringPlaceholderReplacer https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/string_placeholder_replacer.rb. I've made it that way mainly because of the functionality of removing blank lines. I've also preferred to treat nil the same as empty string - StringPlaceholderReplacer does not subsitute placeholder when value is nil. I could try to apply the changes to StringPlaceholderReplacer, by adding an optional parameter with regex for blank line removal. It seems to be used in project badges and mr commit suggestion templates. I think the variables there are never equal to nil anyway, so maybe that change could be moved there as well. I'm not sure which approach is better.

Screenshots or screen recordings

No user-facing changes.

How to set up and validate locally

  1. Go to project settings.
  2. Fill in Merge commit message template or Squash commit message template.
  3. Create a new MR.
  4. Check that the default message in mr widget renders correctly.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Piotr Stankowski

Merge request reports