Wording change: Not sure what this message means

dwyl / dwylbot

:robot: Automating our GitHub Workflow to improve team communication/collaboration and reduce tedious repetition!

28 stars 7 forks source link

Wording change: Not sure what this message means #160

Open iteles opened 6 years ago

iteles commented 6 years ago

Steps:

Cleo assigned this PR to me and Simon: https://github.com/dwyl/dwyl-site/pull/312
I reviewed, requested changes and assigned it back to Cleo
dwylbot gave my this:

I'm assuming what this means is that there is an expectation that the person who opens the PR assigns it for review to someone who isn't themselves. But this is really really unclear from the wording. I was just like... my review is awaiting Cleo's review, why is she the wrong assignee?

Assigning to Mark as our wordsmith.

iteles commented 6 years ago

Also, in our readme it states that this dwylbot message should only appear when there are no assignees on the PR (which there were in this case, Cleo was assigned). So it makes even less sense now I've gone looking for the answer 😬

@markwilliamfirth Is the readme up to date?

ghost commented 6 years ago

@iteles I think dwylbot may have acted correctly here - I think in this case you should have removed the awaiting review label before assigning back to @Cleop

[ ] The readme needs to be updated to reflect the current rules
[ ] This rule needs to be split into two rules
[ ] text needs to be updated to:

⚠️ @#{login}, this pull request has an awaiting-review label, but it doesn't have an assignee! 😱😱😱 Please assign someone to review it! 🙏 👀

⚠️ @#{login}, this pull request has an awaiting-review label, but you are still assigned! 😱😱😱 Please assign someone else to review it! 🙏 👀

Cleop commented 6 years ago

I suppose this depends on the interpretation of the awaiting-review label... to me I see the label as one that can never be assigned to the PR opener but only to a 'Reviewer' because as the person who wrote the code you shouldn't review (and then merge) your own work. When work is assigned back to me to change or discuss I then normally assign the in-progress label so that any other reviewers who have been assigned know not to touch this PR. However, I can also see that it wouldn't be appropriate for @iteles to apply the in-progress label and assign to me when I'm not yet working on the issue. So I'm not sure what label I would advocate. The only existing one I can think of is discuss, however I don't think this one has been used in this context before.