mergify icon indicating copy to clipboard operation
mergify copied to clipboard

Removing Stale Reviews and Labels

Open cipher1024 opened this issue 7 years ago • 13 comments

From the documentation, I see that the following:

pull_request_rules:
  - name: remove outdated reviews
    conditions:
      - base=master
    actions:
      dismiss_reviews:

can be used to dismiss reviews when someone pushes a commit to the PR. If I try to use the same logic to dismiss labels at the same time:

  actions:
      dismiss_reviews:
        approved: True
      label:
        remove: ['ready-to-merge']

the label ready-to-merge gets deleted as soon as it is set.

cipher1024 avatar Apr 02 '19 16:04 cipher1024

Yeah, dismiss_review is triggered only on new commits. Label removal is triggered on any new events, including adding a label.

You can't trigger Mergify actions based on events, only on conditions (state). It just happens than dismiss_reviews has an event condition builtin.

That's something we could maybe enhance by adding a new fields "events" which would allow to filter conditions based on events received. Food for thought, @sileht?

jd avatar Apr 02 '19 18:04 jd

That sounds like a nice improvement. And it would also make clearer when dismiss_review occurs

cipher1024 avatar Apr 02 '19 19:04 cipher1024

We are looking at something similar. See the mention above. We would like to dismiss reviews and do some other actions (remove labels, add labels).

0xc0170 avatar Jan 20 '20 14:01 0xc0170

IIUC @0xc0170 You want to be able to trigger actions based on a condition which would be "new commits since last review" or something like that?

jd avatar Jan 20 '20 15:01 jd

IIUC @0xc0170 You want to be able to trigger actions based on a condition which would be "new commits since last review" or something like that?

Yes. Our use case: if PR is in "needs work" and it was updated(new commits/rebase), it should go into "ready for review" stage (as reviews were dismissed). To move it to a new stage, we use labels, so action for this would be: remove work label and add review.

0xc0170 avatar Jan 20 '20 16:01 0xc0170

An additional tidy up which is similar to what @0xc0170 mentioned is that if a PR is closed rather than merged we would like to be able to remove all labels matching a specific regexp. We use labels to tag which release version a PR would go to E.g. 'release-version: x.y.z' however the label remove function seems to require you to specify exact matching labels which we would not know for any given PR. Is there some way of doing this currently or is that effectively another enhancement ?

adbridge avatar Mar 18 '20 12:03 adbridge

Is there some way of doing this currently or is that effectively another enhancement ?

Yeah, that'd be another enhancement. An easy one would be to delete all labels. Would that work for your use case?

(Deleting only certain label swould be also possible, but more expensive — there's a need to list the label first.)

jd avatar Mar 18 '20 13:03 jd

@jd thanks for the quick response, actually deleting all the labels in that scenario would also work well.

adbridge avatar Mar 18 '20 13:03 adbridge

@adbridge there's now (https://github.com/Mergifyio/mergify-engine/pull/824) a remove_all option for the label action: https://doc.mergify.io/actions.html#label

jd avatar Mar 19 '20 17:03 jd

Great! Many thanks, that will be very useful !

adbridge avatar Mar 20 '20 11:03 adbridge

@jd so if I read the docs correctly would this be the right construction ? :

  - name: remove labels if PR closed but not merged
    conditions:
      - closed
      - -merged
    actions:
      label:
        remove_all=True

adbridge avatar Mar 23 '20 13:03 adbridge

No, this is not a valid YAML syntax. This is more likely what you want:

  - name: remove labels if PR closed but not merged
    conditions:
      - closed
      - -merged
    actions:
      label:
        remove_all: true

jd avatar Mar 23 '20 15:03 jd

Ah I'm still learning the YAML, many thanks

adbridge avatar Mar 23 '20 15:03 adbridge