-
Notifications
You must be signed in to change notification settings - Fork 22
ARC Contributor Guidelines
The purpose of this document is to formalize the guidelines for contributing to ARC. ARC is an open-source project built and maintained by a team of developers. Anyone can contribute to ARC, whether to the code, documentation, or general ideas.
For new developers, the best way to start contributing is by reviewing pull requests. This will help you become more familiar with the codebase and the process of making changes. Another good idea is to start by writing unit-tests for relatively simple functions, contributing toward increasing the code coverage.
The general process and responsibilities for submitting contributions are summarized in the following table. The person making the changes and pull request is the Author, and the person providing feedback on the pull request is the Reviewer. Once the pull request has been approved by the Reviewer, then the request can then be merged. The Merger can be anyone with push permissions to the master branch, aside from the Author. New Mergers can be nominated and approved by the committee of project owners.
| Step | Task | Person Responsible |
|---|---|---|
| 0 | Propose changes and get approval (for big changes) | Author |
| 1 | Implement and test desired changes locally | Author |
| 2 | Make pull request when ready | Author |
| 3 | Request reviewer | Author |
| 4 | Review changes and provide feedback | Reviewer |
| 5 | Address reviewer feedback | Author |
| 6 | Confirm that all checks pass and approve request | Reviewer |
| 7 | Merge pull request | Merger |
Steps 4 and 5 may be repeated as needed until the Reviewer is completely satisfied. The following section provides further details on the required checks and suggested guidelines.
Can you reproduce the issue? Try to reproduce your bug using a recent version of ARC, to see whether it has already been fixed or not. Has someone already reported this issue? Check the Issues page to see if your bug has already been reported, or the feature requested. If you are unsure whether a bug has already been reported, you should report the bug.
- Be precise
- Explain how to reproduce the problem
- Include a minimal input file / API example that reproduces the problem
- Include (the relevant part of) the output file
The primary method to contribute to the ARC codebase is via pull requests. Once a contributor has developed new features or fixes and wishes to merge them into the official codebase, they may create a pull request on GitHub to merge their changes into the master branch.
There are a few mandatory checks that must pass for new pull requests. These should be checked by the Author before making the pull request, and confirmed by the Reviewer before approving the pull request.
-
Unit tests:
The pull request must pass the test suite within the ARC repository, which is run automatically by TravisCI. -
Code coverage:
The pull request should not reduce code coverage by unit tests, which is checked automatically by CodeCov.
Aside from the mandatory requirements, there are a number of other guidelines to ensure that the pull request process proceeds smoothly.
- For big changes/features, the Author should first propose the changes before proceeding, either at a meeting or online (GitHub, Slack, email,), and get approval from the developers that the implementation approach is beneficial for ARC and compatible with the code organization design.
- The Author should make sure that the changes are sufficiently polished and tested before making the pull request.
- A pull request should include a cohesive set of commits implementing a single patch or feature.
- Try to keep pull requests small; it makes reviewing and merging much smoother. However, do not split pull requests in a way that breaks functionality.
- Explain the reason for the pull request and an overview of what the changes are. Reference any issues which the pull request fixes.
- It is the responsibility of the Author to find a reviewer if no one volunteers.
- The Author should ensure that the branch is up to date by rebasing onto the master branch. In particular, this must be done before the pull request is merged.
- Add or modify the docstrings as appropriate, following the Google style
- Make changes to ARC's documentation, as needed.
Use clear and concise commit title should follow, along with a detailed commit description.
Certain commits should start with a special keyword:
-
Docs:for commits that modify the documentation -
Tests:for commits that add/modify tests -
Minor:for minor changes -
BugFix:for bug fixes
The commit message should follow the keyword. Such commits should only contain the relevant content and not additional code changes.
We try to adhere to the Google Style Python Docstrings to facilitate autodoc for the API.
- New contributors are encouraged to review pull requests to gain experience. Depending on the complexity of the changes, a second review by a more experienced contributor might be necessary.
- At least one other contributor must review and approve the pull request. If there are multiple authors, the pull request should be reviewed by a non-author, if possible.
- Any interested person should also contribute to the discussion regarding a pull request. They are encouraged to contribute within a reasonable time-frame, but also have the right to request additional time for discussion.
- Reviewer checklist:
- Does the code work?
- Does the code make sense?
- Are the changes unit-tested?
- Is the code sufficiently commented?
- Are documentation changes necessary?
- Are the commit messages descriptive?
- Pull requests may be merged by anyone who has push access to the master branch, aside from the Author.
- The pull request branch should be rebased on top of the master branch before merging to ensure a clean history. If the branch is not up to date, the Author should rebase it.
- “Merge pull request” option is the preferred way to merge pull requests. “Rebase and merge” option is acceptable if the pull request only contains one (1) commit. “Squash and merge” is never allowed.
Having lots of unused and potentially broken code leads to difficulty developing and using the software. In addition, removing parts of the code which other people depend upon can be detrimental to development and usage as well. A good deprecation procedure can help find a happy optimum.
If you find code which you think is no longer working, you can create an issue about it, using the tag "Type: Deprecation" and/or you can add a deprecation warning to the function or method. A good deprecation warning describes the functionality being deprecated and the next release of ARC which the method/functionality might be removed in.
warnings.warn('The option save_file is no longer supported '
'and may be removed in version 2.3.', DeprecationWarning)
Remember to import the warnings package in the file you are adding the warning to.
This deprecation warning serves as a reminder for people who use this method to mention, on GitHub, that they are potentially deprecating an important functionality for you, which can be the start of a conversation to find the optimum way to improve the code, which could involve refactoring the code, still deprecating the code, or leaving the code as-is.
If no one raises an issue with the change after 6 months, someone can make a pull request removing the method/functionality.