Versions Compared

Old Version 12

changes.mady.by.user Bhavin Thaker

Saved on

compared with

New Version 13

changes.mady.by.user Bhavin Thaker

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

  • 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 but NOT agreed upon yet: 

    • a) It is NOT recommended for a committer to merge pull requests that the committer authored. Instead the committer MUST get at least one approval from another committer to merge his/her pull request.

    • b) When you update a pull request with upstream, you MUST use rebase to ensure that the pull request is easy to review by the community. See the how-to link here: https://mxnet.incubator.apache.org/community/contribute.html

...