Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Added a note on when to delete GitHub comments

...

  • 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.