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

Feature: automatic awaiting review label upon reviewer added to PR #62

Closed
ghost opened this issue Jun 13, 2017 · 18 comments
Closed

Feature: automatic awaiting review label upon reviewer added to PR #62

ghost opened this issue Jun 13, 2017 · 18 comments

Comments

@ghost
Copy link

ghost commented Jun 13, 2017

When a PR is assigned to someone then add "awaiting-review" label on the PR and on the issues listed in the description of the PR

[changed to when a reviewer is added to a PR]

@ghost ghost added the priority-2 label Jun 13, 2017
@ghost ghost assigned SimonLab Jun 13, 2017
@SimonLab
Copy link
Member

SimonLab commented Jun 13, 2017

Instead of using the "assignee" property I think we should use the "reviewer" property. I sometimes assign my PR to myself when I need to update it or when it's still in progress.
Otherwise to resolve this issue:

  • configure dwylbot app on Github to send event when a PR produce event
  • catch the PR events and check for errors (similar to the issues logic)

@SimonLab SimonLab added the T4h label Jun 13, 2017
@ghost
Copy link
Author

ghost commented Jun 13, 2017

@SimonLab sounds good 👍

@SimonLab
Copy link
Member

SimonLab commented Jun 20, 2017

I'm wondering now if this rule is still needed, I don't know if it will be applied often, let's test it.
So if a reviewer is added to the PR the "awaiting-label" is added and the reviewer is added as an assignee of the PR.
Adding "awaiting-review" to the issues linked in the PR required more work and might be better done in it's on issue

@Cleop
Copy link
Member

Cleop commented Jun 21, 2017

@SimonLab - I'm just adding this one to the table of rules, is there meant to be a dwylbot comment with this one to let you know that the awaiting-review label and reviewer have been assigned?

Also, what happens if a reviewer (who has also been assigned) removes the "awaiting-review" label and puts on the "in-review" label. Will dwylbot add the "awaiting-review" label again?

@SimonLab
Copy link
Member

@Cleop At the moment I decided to not add a dwylbot comment I thought it will be to noisy on the PR.

@Cleop
Copy link
Member

Cleop commented Jun 21, 2017

image
Worked in testing for assigning you when I'd made you a reviewer. Interestingly, it added the 'awaiting-review' label even though I had done so the minute before... this didn't break anything though.

I'll leave this for @markwilliamfirth to test properly as he's been assigned but you can check out this screenshot in case its of interest.

@SimonLab
Copy link
Member

SimonLab commented Jun 21, 2017

@Cleop thanks for testing 👍. Maybe dwylbot could test if the "awainting-review" has been added, not a blocker at the moment. I'm still not sure about this rule I'm creating a new issue to discuss it see #116 .

@ghost
Copy link
Author

ghost commented Jun 21, 2017

Tested by Cleo 👍

@SimonLab
Copy link
Member

From @markwilliamfirth on this comment (#129 (comment)):

Change message to:
@username, the in-progress label has been removed from this pull request and a Reviewer has been added. It appears that this pull request is ready for review, so the Reviewer has been added as an Assignee and the awaiting-review label has been applied automatically.

@ghost
Copy link
Author

ghost commented Jul 18, 2017

@SimonLab I was reminded by this: dwyl/bestevidence#40 (comment)

Can we update the comment to this?

screen shot 2017-07-18 at 13 27 10

naazy added a commit that referenced this issue Aug 9, 2017
@ghost
Copy link
Author

ghost commented Aug 9, 2017

@naazy good point:

@username, hoorah! 🎉 It's review time! 👀

I couldn't help but notice that there isn't an in-progress label on this pull request and a Reviewer has been added...makes me think that this pull request is ready for review 🤔

To save you time ⏳ I've added the Reviewer as an Assignee and I've added the awaiting-review label - automatically - just like magic! 🎩 🐰 ✨ . Please correct me if I'm wrong, but if I got it right this time I hope it helps you! 😄

@ghost
Copy link
Author

ghost commented Aug 9, 2017

Linked to #105 (comment)

@ghost
Copy link
Author

ghost commented Aug 9, 2017

@naazy still appears to post old message #146 (comment)

Does the app need to be redeployed before testing can occur?

@ghost ghost removed the please-test label Aug 9, 2017
@ghost ghost assigned naazy and unassigned ghost Aug 9, 2017
@ghost ghost added question and removed T25m labels Aug 9, 2017
@naazy
Copy link
Member

naazy commented Aug 9, 2017

Sorry @markwilliamfirth I thought we had automatic deployment setup. Have spoken to @SimonLab who said he will deploy this evening

@naazy naazy added the in-review label Aug 9, 2017
@ghost ghost removed the question label Aug 9, 2017
@naazy naazy assigned ghost and unassigned naazy Aug 10, 2017
@naazy
Copy link
Member

naazy commented Aug 10, 2017

Ready to test again @markwilliamfirth 🎉

@ghost ghost changed the title Feature: automatic awaiting review label upon assigned PR Feature: automatic awaiting review label upon reviewer added to PR Aug 10, 2017
@ghost
Copy link
Author

ghost commented Aug 10, 2017

This works! Nice one @naazy and @SimonLab 🎉

dwyl/dwylbot-test#4 (comment)

@ghost ghost closed this as completed Aug 10, 2017
naazy added a commit that referenced this issue Aug 18, 2017
@jammur jammur unassigned ghost Dec 14, 2017
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants