'Apply suggestion' should add '!fixup' comments that prevent merging MR without squashing
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
Release notes
Add applied suggestions as 'fixup' commits to force squashing them
Problem to solve
As a code maintainer, I want 'Apply suggestion' to keep Git history clean.
Currently using 'Apply suggestion' during code review creates commits with title 'Apply 1 suggestion(s) to 1 file(s)'. This produces a rubbish commit, violating nearly all rules of creating commits: the title is extremely unhelpful and contents are too small to have a meaning.
I followed steps above and the ugly result looks like this: pkubowicz/reproducing-bugs!4 (commits)
'Apply suggestion' feature encourages creating multiple small meaningless commits on top of real changes and when the MR is merged, the pipeline will show 'Apply 1 suggestion(s) to 1 file(s)' (instead of something related to the MR itself, which happens usually), so you won't know if these are dangerous changes that break backward compatibility or something totally safe.
There is nothing in GitLab that warns the person merging a MR that those added commits should be squashed. It's too easy to add rubbish to Git history even if you have good intentions.
As a consequence, currently I am introducing a rule in my team that forbids using 'Apply suggestion' feature and I am changing scripts in .gitlab-ci.yml
to fail a build if you use 'Apply suggestion', because we agreed that this feature encourages incorrect behaviour.
Intended users
User experience goal
The user should know how to review and merge Merge Requests in GitLab
Proposal
No need to change the UI.
Each applied suggestion should create a Git commit starting with fixup!
. User should not be able to remove this prefix when changing the commit text in 'Apply suggestion' dialog. User input should define text added after the first line of the commit message, not the whole commit message.
This should integrate nicely with existing GitLab functionality. For example, will prevent merging a MR with such commits, as it will become draft (see Mark merge requests as drafts in the documentation). Only after those small commits are squashed a reviewer will be able to merge.
Further details
Alternatively, adding suggestion could enable 'squash' option for a MR, but this would be complicated, as there are repository-level settings on how squashes are handled. I think the option with 'fixup' is better.
Permissions and Security
Documentation
Availability & Testing
Available Tier
What does success look like, and how can we measure that?
For a MR that has 'sqush' option disabled, a MR reviewer is not able to apply a suggestion and then immediately merge the MR.
When MR reviewers act in good faith, commits with title 'Apply 1 suggestion(s) to 1 file(s)' will never appear in Git repository.