...
As a reviewer you have the responsibility to ensure contributions meet the quality bar. This means you should try and ensure:
Contributions are inline with the overall code architecture.
Contributions are not lowering the overall code maintainability i.e. documentation and unit tests are not missing.
If you don’t understand the logic/flow now, chances are it won’t automatically get better in the future. So feel free to push back if code/logic is hard to follow. Ask to include comments clarifying implementation details whenever necessary.
When providing feedback or when seeking clarification, be clear. Don’t assume the PR owner knows exactly what is in your mind.
If you decide to close a PR without merging, get an ack from the PR owner that that is indeed the right next step.
Proposed NOT agreed upon yet:
a) A committer should NOT delete a comment from any contributor. But in the rare case, a comment has to deleted, it must be approved by at least 3 +ve votes with no -ve votes for 24 hours.
b) If committer self-merges code developed by self, it MUST contain the name of another committer who reviewed the code.
- c) All conversations and comments must be relevant, respectful and professional.
- d) Need some guidelines Guidelines on when to rebase and when to merge??rebase: https://mxnet.incubator.apache.org/community/contribute.html