Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing Stale Reviews and Labels #360

Open
cipher1024 opened this issue Apr 2, 2019 · 13 comments
Open

Removing Stale Reviews and Labels #360

cipher1024 opened this issue Apr 2, 2019 · 13 comments

Comments

@cipher1024
Copy link

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.

@jd
Copy link
Member

jd commented Apr 2, 2019

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?

@cipher1024
Copy link
Author

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

@0xc0170
Copy link

0xc0170 commented Jan 20, 2020

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).

@jd
Copy link
Member

jd commented Jan 20, 2020

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?

@0xc0170
Copy link

0xc0170 commented Jan 20, 2020

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.

@adbridge
Copy link

adbridge commented Mar 18, 2020

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 ?

@jd
Copy link
Member

jd commented Mar 18, 2020

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.)

@adbridge
Copy link

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

mergify bot pushed a commit that referenced this issue Mar 19, 2020
This new option allows to remove all labels of a pull request.

Relates to #360
@jd
Copy link
Member

jd commented Mar 19, 2020

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

@adbridge
Copy link

Great! Many thanks, that will be very useful !

@adbridge
Copy link

@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

@jd
Copy link
Member

jd commented Mar 23, 2020

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

@adbridge
Copy link

Ah I'm still learning the YAML, many thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants