...
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) Self-merge by a committer is NOT recommended, but if required in a rare scenario, the PR MUST contain the name of another committer who reviewed the code before it is merged.
- c) All conversations and comments must be relevant, respectful and professional.
- d) Guidelines on rebase: https://mxnet.incubator.apache.org/community/contribute.html
Deleting Comments on GitHub
GitHub issues and pull requests allow a user with "Write" permissions to delete another's comment. The Apache Way yearns for openness in all situations, deleting a comment is rarely the right thing to do.
If a comment does need deleting, most likely because it does not adhere to our Code of Conduct, then the PMC as a whole should discuss and vote on the deletion. If a comment needs to be deleted quickly, then either the PMC Chair (or while in the Incubator a Mentor) should make that decision.