search

dwyl / contributing

:clipboard: Guidelines & Workflow for people contributing to our project(s) on GitHub. Please :star: to confirm you've read & understood! :white_check_mark:
86 stars 9 forks source link

What to do once you have addresses a reviewer's comments #102

Open iteles opened 6 years ago

iteles commented 6 years ago

I have recently seen a couple of instances at dwyl where after addressing a reviewer's comments, the original creator of the PR merges their own PR without passing this back to the reviewer to ensure their comments have been satisfactorily addressed.

It has brought to my attention that this section in our documentation does not address this situation and needs to be made more explicit: https://github.com/dwyl/contributing#step-4-reviewers-review-comment-on-and-merge-the-pull-request

ghost commented 6 years ago

@iteles this PR was approved (request changes wasn't selected) - is that still the case here?

https://github.com/dwyl/process-handbook/pull/62

iteles commented 6 years ago

Exactly why this needs to be clarified, it's a great question.

One never merges in one's own PR. In this specific case, a reviewer may leave an approved PR open until they are satisfied that the original creator of the PR has acknowledged their comments (by opening issues or a comment on the PR if appropriate) and then the reviewer merges it in.

In other cases, there have been instances of PRs not being approved and then someone merging in their own PR once they have made those changes. Or even of a reviewer adding in some commits of their own - it is not clear from our current processes what happens in any of these 3 instances at the moment.