This page captures the approach we'll follow for streamlining the development process in MXNet project.

Pull Requests

As MXNet contributions continue to grow there is a need to scale the capacity for reviewing incoming PRs. While the primary goal is to have a subject matter expert (SME) review the PR and provide timely feedback, a secondary goal is to grow the pool of SMEs and farm out the ownership of reviewing specific PRs to them. As a first step, we will have a list of volunteers who have a demonstrated proficiency and a track record of making quality contributions to various MXNet components as SMEs for those areas.

...

1. Identify a set of MXNet components and for each component a list of reviewers who are willing to review submissions related to that component. For example, the components could be “engine”, “operators”, “tutorials”, “python/scala/R bindings”, etc. Alternatively, since the code base is already organized into logical folders, we can identify a set of reviewers to each source folder..

2. When creating a new PR, the best practice is to label the PR with appropriate components being modified. Optionally, tag the reviewers for those components in the PR.

3. Core MXNet maintainers such as @piiswrong and @mli can always pitch in and provide additional feedback on any PR or tag specific other developers whose feedback might be useful.

Guidelines for Contributors

1. Fork the Github repository at https://github.com/apache/incubator-mxnet if you haven’t already
2. Clone your fork, create a new branch, push commits to the branch.
3. Coding and Documenting
   1. Consider whether documentation or tests need to be added or updated as part of the change, and add them as needed.
   2. When adding new APIs or modifying existing APIs:
      

      1. Make sure to add/update appropriate API documentation.

      2. If adding a new API, include in the API documentation sample code that shows how to use the API.

      3. If modifying an existing API, make sure that API documentation and code samples are still accurate after your changes.

      4. Changes that add new functionality or extend existing functionality should always be accompanied by unit tests that exercise that functionality.

      5. When updating code that is performance sensitive, include in the PR description details on how you tested your changes from a performance standpoint.

4. Open a pull request against the master branch of apache/mxnet. (Only in special cases would the PR be opened against other branches.)
   
   1. The PR title should be of the form [MXNET-xxxx] Title, where MXNET-xxxx is the relevant JIRA number, title may be the JIRA’s title or a more specific title describing the PR itself.
      1. only the very tiny fix, e.g. fix a typo, remove some unused variables, or the hotfix which brings the broken build back to track, can be filed without JIRA ID in title,
   2. If the pull request is still a work in progress, and so is not ready to be merged, but needs to be pushed to Github to facilitate review, then add [WIP] after the component.
   3. Consider identifying committers or other contributors who have worked on the code being changed. Find the file(s) in Github and click “Blame” to see a line-by-line annotation of who changed the code last. You can add @username in the PR description to ping them immediately.
   4. Please state that the contribution is your original work and that you license the work to the project under the project’s open source license.
5. There is no need to be the Assignee of the related JIRA to work on it, though you are welcome to comment that you have begun work.
6. The Jenkins automatic pull request builder will test your changes
7. Jenkins will update the results of the test in the pull request
8. Watch for the results, and investigate and fix failures promptly
   1. Fixes can simply be pushed to the same branch from which you opened your pull request
   2. Jenkins will automatically re-test when new commits are pushed
   3. If the tests failed for reasons unrelated to the change (e.g. Jenkins outage), you are supposed to ping the dev mail list about the outage or file a JIRA regarding the flaky test
9. When you receive feedback for your PR, try and address the reviewer’s comments/concerns in a timely fashion. If you don’t have time to immediately address them, adding a short note to the PR about when you plan to update the PR will be helpful. Feel free to solicit help if you want others to pitch in.

Guidelines for Reviewers/Committers

1. As a reviewer you have the responsibility to ensure contributions meet the quality bar. This means you should try and ensure:

   1. Contributions are inline with the overall code architecture.

   2. Contributions are not lowering the overall code maintainability i.e. documentation and unit tests are not missing.

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

2. When providing feedback or when seeking clarification, be clear. Don’t assume the PR owner knows exactly what is in your mind.

3. If you decide to close a PR without merging, get an ack from the PR owner that that is indeed the right next step.

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

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

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. 

...