Remove draft markers only at the start and end of a title

What does this MR do and why?

Gitlab::Dangerfiles::TitleLinting::DRAFT_REGEX starts with \A*. The * lets the anchor match zero times, so the markers a title starts with (Draft:, [draft], (draft), draft - , WIP:, [WIP]) match anywhere in it, and remove_draft_flag removes every one of them with gsub. BaseLinter#subject lints the stripped text, so the commit linter and the merge request title linter check a different subject from the one that was written: Reject draft: titles is linted as Reject titles and reported as too short, and a subject is measured without such a marker when its length is checked.

I anchored those markers at the start of the title, still allowing several in a row (Draft: [WIP] My title still becomes My title), and kept removing a WIP or draft at the end, which the "when used as suffix" examples in base_linter_spec.rb expect. A marker anywhere else is now part of the title and is linted with it.

Closes #106 (closed)

The MR has two commits, and I would like to keep them separate when it is merged:

  1. Pin the draft markers remove_draft_flag strips today: rows in the remove_draft_flag table for what it removes on master and the table did not list ([WIP] and Draft - at the start, stacked markers, a trailing WIP or draft, a title made only of markers). They pass on master and hold the fix to the same results.
  2. Remove draft markers only at the start and end of a title: the anchored pattern, with examples for markers in the middle of a title in title_linting_spec.rb, commit_linter_spec.rb and merge_request_linter_spec.rb. All 11 new examples fail with the first commit's pattern and pass with this one. It carries Changelog: fixed.

Two things I left as they are:

  • The trailing forms also match an ordinary last word: Save the draft is still linted as Save the . The suffix examples pin that, so I kept it out of this MR, as the issue says.
  • I put the linter examples in commit_linter_spec.rb and merge_request_linter_spec.rb rather than base_linter_spec.rb, which !328 (merged) changes.

The end of the title is matched with \z where it used to be $, so a WIP or draft before a line break is no longer removed. Both linters only read the first line of a message, so this does not change what they report.

How to set up and validate locally

Save this as repro.rb and run ruby -Ilib repro.rb from the repository root, with Ruby 3.2.9 from .tool-versions:

require "gitlab/dangerfiles/commit_linter"

title_linting = Gitlab::Dangerfiles::TitleLinting
p title_linting.remove_draft_flag("Fix the draft: parser")
p title_linting.remove_draft_flag("Show (draft) badge")
p title_linting.remove_draft_flag("Handle a WIP: prefix")
p title_linting.remove_draft_flag("Use draft - mode")

commit = Struct.new(:message, :sha, :diff_parent)
p Gitlab::Dangerfiles::CommitLinter.new(commit.new("Reject draft: titles\n\nBody.", "abc", [])).lint.problems
master (1153e0d5) this branch
Fix the draft: parser "Fix the parser" "Fix the draft: parser"
Show (draft) badge "Show badge" "Show (draft) badge"
Handle a WIP: prefix "Handle a prefix" "Handle a WIP: prefix"
Use draft - mode "Use mode" "Use draft - mode"
Reject draft: titles (commit) subject_too_short no problem

Checks I ran the way .gitlab-ci.yml runs them, in the ruby:3.0, ruby:3.1 and ruby:3.2 images CI uses (Ruby 3.0.7, 3.1.7 and 3.2.11 here) and on Ruby 3.2.9 from .tool-versions:

  • bundle exec rspec: 635 examples after the first commit and 646 after the second, 0 failures on each Ruby version (626 examples on master).
  • bundle exec rubocop -P -E .: no offenses, on each commit and Ruby version.
  • bundle exec danger dry_run, which the pre-push hook in lefthook.yml runs and which compares against origin/master: no messages, so both commit messages pass the commit_messages rule.
The new examples without the fix

The second commit's specs, run with the first commit's title_linting.rb (Ruby 3.2.9; 3.2.11 gives the same list):

646 examples, 11 failures

Failed examples:

rspec './spec/gitlab/dangerfiles/commit_linter_spec.rb[1:6:1:1:1:1]' # Gitlab::Dangerfiles::CommitLinter#lint subject when a draft marker is in the middle of it behaves like a valid commit does not have any problem
rspec ./spec/gitlab/dangerfiles/commit_linter_spec.rb:118 # Gitlab::Dangerfiles::CommitLinter#lint subject when a draft marker in the middle of it makes it too long adds a problem
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:16:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag mr_title: "Draft: Fix the draft: parser", expected_mr_title: "Fix the draft: parser" is expected to eq "Fix the draft: parser"
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:17:1:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag when a draft marker is in the middle of the title mr_title: "Fix the draft: parser" keeps the title as it is
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:17:2:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag when a draft marker is in the middle of the title mr_title: "Show (draft) badge" keeps the title as it is
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:17:3:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag when a draft marker is in the middle of the title mr_title: "Keep [draft] notes" keeps the title as it is
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:17:4:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag when a draft marker is in the middle of the title mr_title: "Use draft - mode" keeps the title as it is
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:17:5:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag when a draft marker is in the middle of the title mr_title: "Handle a WIP: prefix" keeps the title as it is
rspec './spec/gitlab/dangerfiles/title_linting_spec.rb[1:2:17:6:1]' # Gitlab::Dangerfiles::TitleLinting#remove_draft_flag when a draft marker is in the middle of the title mr_title: "Ignore [WIP] markers" keeps the title as it is
rspec ./spec/gitlab/dangerfiles/title_linting_spec.rb:82 # Gitlab::Dangerfiles::TitleLinting#has_draft_flag? returns false for a title with a draft marker after its start
rspec './spec/gitlab/merge_request_linter_spec.rb[1:1:3:1:1]' # Gitlab::Dangerfiles::MergeRequestLinter#lint_subject when a draft marker is in the middle of it behaves like a valid mr title does not have any problem
Comparing the two patterns on random titles

To check that nothing else changed, I ran both patterns through gsub on 300,000 random single-line titles built from marker tokens (Draft:, [WIP], (Draft), draft - , a bare WIP or draft, in several cases) and ordinary words. Whenever master's pattern matched only in the run of markers at the start of the title or a bare WIP or draft at its end, both gave the same result (134,199 titles). Every title where they differed (165,801) had a match master removed from somewhere else, and in each of them the new result is the longer one, with that marker kept.

Merge request reports

Loading
Loading