PR Review standards

[jakirkham]

This has come up in a few places and I think it would be helpful to have a discussion somewhere. So, I have opened this issue. Feel free to add any thoughts you would like here.

[jakirkham]

Some discussion points came up in the hackpad I am adding them here with some points that came to mind so that I don't forget them.

• Treat every PR as a Work in Progress. At least let PRs sit for a few hours before merging them.
  • It is very tricky with so many PRs to tell their importance or state. While waiting for a few hours seems fine. Maybe we can take a look at some other organizational strategies that will keep us from stepping on each others toes. One thing to consider might be how scikit-learn approaches this problem. They suggest adding WIP to PRs still being worked on, MRG to PRs that are ready for thorough review, and MRG+1 to PRs that have been reviewed and have at least one LGTM. ( https://github.com/scikit-learn/scikit-learn/blob/master/CONTRIBUTING.md#contributing-pull-requests )
• Wait for answers when we ask clarification questions and avoid acting before we have them.
  • Not sure where this came up, but some thoughts here might include documenting a checklist of todos/open questions, concerns, and other unresolved issues as is done in this PR ( conda forge/staged recipes#370 ).
• Respect the first reviewer by not repeating her/his review comments with another words. That is also bad for the person submitting the PR as it is confusing.
  • Not sure if I exactly understand this point as written, but I believe this means don't repeat something that people agree with. Maybe we can opt for using the 👍 button to just agree to keep things simple. If this is related to points of disagreement as well, please see the point below (tldr documented standards that we agree on avoids having disagreements on every change).
• Avoid the death by a thousand cuts: Many small "nit" comments that might scare new contributors ( ping Mike S ;-)
  • I think one thing that will help here is that we agree on some standards and document them. ( Starting docs #95 ) ( Maintainer preferences staged-recipes#139 ) ( Create PULL_REQUEST_TEMPLATE.md [skip ci] staged-recipes#550 ) These seem to be forever in flux of late. So, it is really hard to tell what is ok, what is not ok, and whether we even agree.

[ocefpaf]

I added the topics to the next meeting agenda and I would like to discuss there first. before opening any issue here or anywhere. I'd appreciate if you could understand that @jakirkham as there are sensitive issues I want to bring that are not appropriate for discussion here.