Do not accept merge request if contains `WIP` label
Created by: sue445
I use the WIP MR at the beginning of the work
WIP MR should not be merged. But I (or other) submit the accept button by mistake often.
So I hide accept button if MR title include "WIP"
Merge request reports
Closed by
(Mar 9, 2025 9:34am UTC)
Activity
5 5 .panel-body 6 6 - if @merge_request.open? 7 7 - if @merge_request.source_branch_exists? && @merge_request.target_branch_exists? 8 = render "projects/merge_requests/show/mr_accept" 8 - if @merge_request.wip? Created by: jvanbaarsen
@sue445 Can you maybe try and write tests for this feature?
By Administrator on 2014-04-09T18:26:57 (imported from GitLab project)
By Administrator on 2014-04-09T18:26:57 (imported from GitLab)
Unable to load the diff Unable to load the diff Unable to load the diff Unable to load the diff Created by: sue445
How's this? 67d5e95
- extract to method and write test
- strict match "WIP" as a word
- if title contains "WIP" but part of a other word(ex.
Fix wipeout), do not hide accept button
- if title contains "WIP" but part of a other word(ex.
By Administrator on 2014-04-10T17:22:22 (imported from GitLab project)
By Administrator on 2014-04-10T17:22:22 (imported from GitLab)
Created by: jvanbaarsen
@sue445 Can you please make this PR mergeable?
By Administrator on 2014-04-11T07:35:51 (imported from GitLab project)
By Administrator on 2014-04-11T07:35:51 (imported from GitLab)
Created by: kubark42
Just my $0.02 worth but this seems this feature could cause some unintended confusion. I certainly see the value of it, but I wonder if casting the net so wide is necessary. "wip" could stand for others things in addition to "work in progress" and this could cause unexpected behavior.
The same feature could be accomplished by requiring that the exact string
[WIP]appear as the first five characters. Users who want the feature will know the standard for using it, and those who don't are extremely unlikely to chance across it.By Administrator on 2014-04-12T07:09:05 (imported from GitLab project)
By Administrator on 2014-04-12T07:09:05 (imported from GitLab)
Created by: sue445
@kubark42 Hmmm. That you are saying do you these things?
def wip? title.start_with?("[WIP]") end@randx @jvanbaarsen @nkukard What do you think?
By Administrator on 2014-04-12T04:39:01 (imported from GitLab project)
By Administrator on 2014-04-12T04:39:01 (imported from GitLab)
Created by: kubark42
@sue445 Something like that, yes. I like the feature and look forward to using it, but it seems like the pattern matching should err on the side of caution and produce more false negatives than positives.
By Administrator on 2014-04-12T05:42:51 (imported from GitLab project)
By Administrator on 2014-04-12T05:42:51 (imported from GitLab)
Created by: sue445
@kubark42 @jvanbaarsen Modify done. and tweak CHANGELOG
By Administrator on 2014-04-12T08:22:15 (imported from GitLab project)
By Administrator on 2014-04-12T08:22:15 (imported from GitLab)
Created by: jvanbaarsen
@sue445 Thanks! Sadly this MR is not mergeable at this moment, can you please rebase with master and make it mergeable again?
By Administrator on 2014-04-12T08:41:44 (imported from GitLab project)
By Administrator on 2014-04-12T08:41:44 (imported from GitLab)
Created by: sue445
@jvanbaarsen
rebaseandpush -f!By Administrator on 2014-04-13T22:30:54 (imported from GitLab project)
By Administrator on 2014-04-13T22:30:54 (imported from GitLab)
Created by: cirosantilli
@sue445 sorry, but I'm against this feature for the same reasons as exposed by @kubark42 .
If you problem is accidentally clicking on the Merge button, why not require two clicks to merge a MR, either via a confirm popup or by opening up an invisible div like GitHub does now? That, I would agree with.
If we really take this road, why not just create a checkbox and a new database field dedicate to WIP status? Its saner than storing the data as part of strings that have other functions.
By Administrator on 2014-04-14T12:57:44 (imported from GitLab project)
By Administrator on 2014-04-14T12:57:44 (imported from GitLab)
Created by: jvanbaarsen
@cirosantilli I do not agree on that last part, This way its just something thats embedded in the system, without much code complications.
@dosire What do you guys think of this feature.
By Administrator on 2014-04-14T13:26:37 (imported from GitLab project)
By Administrator on 2014-04-14T13:26:37 (imported from GitLab)
Created by: dosire
@jvanbaarsen We discussed it this morning and we'll discuss it again tomorrow. I should post feedback here in 48 hours.
By Administrator on 2014-04-14T13:28:16 (imported from GitLab project)
By Administrator on 2014-04-14T13:28:16 (imported from GitLab)
Created by: dosire
We don't think that editing the title is a user friendly way to do this.
Maybe an option is to add a
Work in Progressnotice to the show page as long as the MR is not assigned to anyone?By Administrator on 2014-04-15T06:48:51 (imported from GitLab project)
By Administrator on 2014-04-15T06:48:51 (imported from GitLab)
Created by: cirosantilli
@dosire We can already see on the show page if a MR is assigned to someone or not, so is it really necessary to duplicate that information? Don't labels + a CONTRIBUTING convention deal nicely with WIP already?
By Administrator on 2014-04-15T07:09:22 (imported from GitLab project)
By Administrator on 2014-04-15T07:09:22 (imported from GitLab)
Created by: sue445
The purpose of this feature is that do not merge by mistake. (me or other person)
There is a culture that add
WIPto PR/MR title in Japan We will discuss the changes and specifications on WIP PR and finally removeWIPand final review.This feature is inspired by Github Merge Caution https://github.com/sanemat/github-merge-caution
By Administrator on 2014-04-15T07:52:59 (imported from GitLab project)
By Administrator on 2014-04-15T07:52:59 (imported from GitLab)
Created by: dosire
@cirosantilli If everyone plays nicely, yes. But I made the mistake of merging a WIP more than once and @sue445 also had that problem. I really like seeing https://github.com/sanemat/github-merge-caution that addresses the same problem.
By Administrator on 2014-04-15T07:54:32 (imported from GitLab project)
By Administrator on 2014-04-15T07:54:32 (imported from GitLab)
Created by: cirosantilli
@dosire merge-caution also requires people to play nicely by adding
[wip]to the title. But since we control the source of GitLab, we could required people to play nice in a saner way =), either via a magic WIP label, or a checkbox + new MR DB column.Just imagine how this scales, if we decide to keep all new MR metadata on the title:
Merge request [META1][WIP][META2]This would be:
- harder and slower to manipulate since we'd have to parse the title string every time.
- harder for new users to learn. They would have to read the manual to discover that the feature exists, while a check box is evident.
Probably one reason why sanemat implements it like that is that GitHub allows only admins to add labels, so people created the practice of adding labels on the title. Fortunately, GitLab lets any user tag his MRs.
True, same tradeoffs of the task list in markdown feedback decision, in which the "embed metadata" option was taken.
By Administrator on 2014-04-15T08:25:18 (imported from GitLab project)
By Administrator on 2014-04-15T08:25:18 (imported from GitLab)
Created by: dosire
@cirosantilli I agree with not adding this to the title. I think the options are:
- Implicitly (not assigning means work in progress)
- With a label
- With a checkbox
Out of these, I prefer the first one since it means the least amount of work.
By Administrator on 2014-04-15T08:28:42 (imported from GitLab project)
By Administrator on 2014-04-15T08:28:42 (imported from GitLab)
Created by: cirosantilli
@dosire I'm against 1. because it breaks the Principle of least astonishment (for me): why can't I merge something that is not assigned?
By Administrator on 2014-04-15T08:42:14 (imported from GitLab project)
By Administrator on 2014-04-15T08:42:14 (imported from GitLab)
Created by: dosire
@cirosantilli You misunderstood https://github.com/gitlabhq/gitlabhq/pull/6739#issuecomment-40450762
We just add a text, you can still merge if you choose to ignore it, which will be common with MR's you merge yourself.
By Administrator on 2014-04-15T08:43:14 (imported from GitLab project)
By Administrator on 2014-04-15T08:43:14 (imported from GitLab)
Created by: jvanbaarsen
@dosire In this case I would go for option 2, a magic label. Since not assigning it has not always worked. I've sometimes merge something without taking a good look. I think blocking the merge somehow is a good thing.
By Administrator on 2014-04-15T08:43:32 (imported from GitLab project)
By Administrator on 2014-04-15T08:43:32 (imported from GitLab)
Created by: cirosantilli
@dosire sorry, I had misunderstood. Its clear now. I feel magic
WIPtag is as easy to implement and more flexible. For e.g., it allows to filter the index separately by either "no one assigned" or WIP, which may happen separately, but I accept whichever you guys think best =)By Administrator on 2014-04-15T11:02:31 (imported from GitLab project)
By Administrator on 2014-04-15T11:02:31 (imported from GitLab)
Created by: dosire
@sue445 What do you think about 1?
By Administrator on 2014-04-16T09:23:14 (imported from GitLab project)
By Administrator on 2014-04-16T09:23:14 (imported from GitLab)
Unable to load the diff Created by: cirosantilli
@sue445 Once #6778 is done, 2. will also be a one liner by changing this line to check if the MR has a label
WIP. Why not wait for it?By Administrator on 2014-05-15T17:21:46 (imported from GitLab project)
By Administrator on 2014-05-15T17:21:46 (imported from GitLab)
Unable to load the diff Created by: sue445
@cirosantilli ok, I'll wait for merge
By Administrator on 2014-05-15T17:21:46 (imported from GitLab project)
By Administrator on 2014-05-15T17:21:46 (imported from GitLab)
Created by: dosire
@sue445 Maybe we can display the text
Work In Progressand have a hover-over text of 'This merge request is not assigned to anyone so it might not be ready for merging yet.`By Administrator on 2014-04-16T11:15:46 (imported from GitLab project)
By Administrator on 2014-04-16T11:15:46 (imported from GitLab)
Created by: jvanbaarsen
@sue445 What is the status of this MR?
By Administrator on 2014-04-30T19:44:32 (imported from GitLab project)
By Administrator on 2014-04-30T19:44:32 (imported from GitLab)
Created by: sue445
@jvanbaarsen I'm waiting !6778 (merged) https://github.com/gitlabhq/gitlabhq/pull/6739#discussion_r11676418
I'll use new MR label feature
By Administrator on 2014-05-03T16:52:40 (imported from GitLab project)
By Administrator on 2014-05-03T16:52:40 (imported from GitLab)
Created by: jvanbaarsen
@sue445 Ok! thanks for your response. Issue is waiting for #6778 to be merged
By Administrator on 2014-05-05T19:23:05 (imported from GitLab project)
By Administrator on 2014-05-05T19:23:05 (imported from GitLab)
320 320 source_project.repository.branch_names 321 321 end 322 322 end 323 324 # whether this is WIP merge request 325 def wip? 326 labels.map(&:name).any? { |label| label =~ /^wip$/i } Created by: dosire
@sue445 We discussed this PR at length with the GitLab B.V. team. We feel that having the workflow for merging an MR that was marked WIP before involves too many clicks (edit, remove WIP label, save, merge). We would like to explore the implicitly flow (not assigning means work in progress) and are welcoming prototype screenshots of the state where it is not assigned yet (probably contains a WIP warning and an 'Assign to me and merge' button').
By Administrator on 2014-05-19T07:48:45 (imported from GitLab project)
By Administrator on 2014-05-19T07:48:45 (imported from GitLab)
Created by: dosire
@sue445 This is not the implicit flow I suggested in my previous comment.
By Administrator on 2014-05-29T13:14:19 (imported from GitLab project)
By Administrator on 2014-05-29T13:14:19 (imported from GitLab)
Created by: jvanbaarsen
@sue445 The flow that @dosire means, is that there is absolutely no locking on the Merge Request.
Maybe you can show a warning on the merge page if the MR is not yet assigned, with something like: "This merge request has not yet been assigned."
MR's where there will be MR locking will not be accepted. That being said, thanks for all your work!
By Administrator on 2014-05-30T18:22:34 (imported from GitLab project)
By Administrator on 2014-05-30T18:22:34 (imported from GitLab)
Created by: dosire
@jvanbaarsen Thanks for explaining in detail.
@sue445 What @jvanbaarsen said is correct. We do greatly value you contributions. I love https://github.com/sue445/gitpeach
By Administrator on 2014-06-02T10:46:56 (imported from GitLab project)
By Administrator on 2014-06-02T10:46:56 (imported from GitLab)
Created by: sue445
@dosire @jvanbaarsen Thank you for explain. I think other better way. Should I close this PR once?
thx
By the way, I made another gitlab util too.
By Administrator on 2014-06-03T16:41:00 (imported from GitLab project)
By Administrator on 2014-06-03T16:41:00 (imported from GitLab)
Created by: dosire
@sue445 Cool, I already saw it before but now I added it to the readme https://gitlab.com/gitlab-org/gitlab-ce/blob/master/README.md#resources
I think you can close this PR unless you want to implement the other way.
By Administrator on 2014-06-03T17:14:48 (imported from GitLab project)
By Administrator on 2014-06-03T17:14:48 (imported from GitLab)
Created by: jvanbaarsen
@sue445 I'll close this PR for now. If you think it should stay open, please let me know with the reason. Thanks for all the efforts!
By Administrator on 2014-07-09T18:54:19 (imported from GitLab project)
By Administrator on 2014-07-09T18:54:19 (imported from GitLab)
Created by: jvanbaarsen
@sue445 Ok, thank you so much!
By Administrator on 2014-07-12T08:21:51 (imported from GitLab project)
By Administrator on 2014-07-12T08:21:51 (imported from GitLab)
