Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

New workflow for "pr-handover" #32

Open
rhuss opened this issue Dec 6, 2017 · 4 comments
Open

New workflow for "pr-handover" #32

rhuss opened this issue Dec 6, 2017 · 4 comments

Comments

@rhuss
Copy link
Contributor

rhuss commented Dec 6, 2017

@zregvart and I tested yesterday a workflow for a PR which I approved in general but had one comment which might or might not be addressed (so 'nice to have'). If I would have approved it directly, then the PR gets merged immediately with no chance for the author to address the comment if he wants (he would have to open a new PR). On the other hand, I don't want to come back to the PR if there is no change. So I added a "WIP" label. The author then could either change his code because of the comment (then the Review status is reset and it would be awesome if the "Approved" label would be removed, too). Or, if he still wants to have it merged fast just remove the "WIP" label, then the PR gets merged automatically. Some sort of 'handover' of the PR back to the original author.


My idea is to extend pure-bot:

  • If there is a "[handover]" in an extra line (e.g. with regexp /^\s*[+?\s*handover\s]+/mi ) in the approval comment, then pure-bot should apply a label "pr/handover" in addition to the "approved" label.
  • This label should have the same semantics as "status/wip" : When present don't merge, if absent (and "approved" is there) then merge.

Thinking about it, I would consolidate the labels with semantics for pure-bot to:

  • "approved" --> "pr/approved"
  • "status/wip" --> "pr/wip"
  • ... -> "pr/handover"

Happy for other naming suggestion, though.

@syndesis/all wdyt ?

@rhuss rhuss changed the title New Workflow for "pr-handover" New workflow for "pr-handover" Dec 6, 2017
@zregvart
Copy link
Member

zregvart commented Dec 6, 2017

Perhaps we can be more explicit for the pr/handover label, something like pr/remove-to-merge? And perhaps we can use [ok to merge] (or 👌 to be real hipsters) instead of [handover]?

@rhuss
Copy link
Contributor Author

rhuss commented Dec 6, 2017

Yeah, I'm perfectly fine with that as it is more explicit.

@rhuss
Copy link
Contributor Author

rhuss commented Mar 27, 2018

Latest thoughts on this feature (from syndesisio/syndesis#1914 (comment)):

  • Reviewer reviews, finds some minor issues which might or might not be fixed, but in general he's fine with the PR and wants to finish the review
  • He does not approve, but just comment (but also don't request changes)
  • He adds a label "pr/approved-with-handover" pure-bot monitors this label, and as soon when this label is removed, it applies a "pr/approved" label. This will then trigger a merge.
  • That way the author can still adapt the PR (or not) and just removes the label when its done.

Currently, "pr/wip" could be used, too, but this is not so transparent for people looking at such a PR.

@rhuss
Copy link
Contributor Author

rhuss commented Mar 27, 2018

The actual label should be configurable, though.

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

No branches or pull requests

2 participants