|
| 1 | +# Astropy Project Pull Request Review & Merging Policy |
| 2 | + |
| 3 | +## Astropy Core Package |
| 4 | + |
| 5 | +When submitting a pull request (PR) in the Astropy core package the following |
| 6 | +process must take place before it can be accepted. If the pull request fails any |
| 7 | +of the reviews or checks it must be amended and undergo additional review before |
| 8 | +being accepted. |
| 9 | + |
| 10 | +### Review & Approval |
| 11 | + |
| 12 | +All pull requests will be reviewed by at least one contributor with relevant |
| 13 | +expertise to ensure that: |
| 14 | + |
| 15 | + - New or updated code is correct and achieves the desired effect of |
| 16 | + either fixing a bug or making an enhancement. |
| 17 | + - New tests or modifications to tests are adequate to cover the code changes. |
| 18 | + Tests for edge cases exist where applicable and judged to be sufficiently |
| 19 | + important. |
| 20 | + - There are no API changes or the API changes are well-understood and |
| 21 | + acceptable and beneficial on the whole to the Astropy user community, and |
| 22 | + are documented as an API change in the changelog. |
| 23 | + - Formatting and style maintain the existing style of a package and |
| 24 | + readability of the code base for future developers. |
| 25 | + |
| 26 | +Reviews will be detailed and should include review and suggestions at the |
| 27 | +line-by-line level, although "I looked and had no comments" is a valid answer |
| 28 | + |
| 29 | +For pull requests that require specific knowledge of a sub-package, a maintainer |
| 30 | +of the sub-package that the contribution affects is required to approve. For |
| 31 | +other less complex cases, for example a documentation improvement, review may be |
| 32 | +conducted by any Astropy core package maintainer. |
| 33 | + |
| 34 | +While a core package maintainer does not have to be the one performing the |
| 35 | +review, if the review is performed by someone else, that maintainer is |
| 36 | +responsible for ensuring the contributor review is sufficiently detailed and |
| 37 | +follows these guidelines. |
| 38 | + |
| 39 | +After successful review, the maintainer(s) will formally approve the pull |
| 40 | +request. |
| 41 | + |
| 42 | +### Additional Checks |
| 43 | + |
| 44 | +Additional checks will be performed to verify that there are no other issues |
| 45 | +with the pull request, for example: |
| 46 | + |
| 47 | +- A release manager or maintainer will check that the selected release milestone |
| 48 | + is suitable for the scope of the change. In particular bug-fix backports are |
| 49 | + considered where feasible and sensible. Documentation is usually backported. |
| 50 | + API changes or new feature changes are not backported unless under special |
| 51 | + circumstances. |
| 52 | +- A matrix of CI checks will be run to ensure that all existing tests pass on |
| 53 | + supported platforms. Even CI jobs that are allowed to fail should still pass |
| 54 | + unless a known and unrelated failure occurs; i.e., look at the logs even in |
| 55 | + the event of a green check mark. |
| 56 | +- An automated tool will check that the change log mentions the change and PR |
| 57 | + number and that the change log section used matches the selected release |
| 58 | + milestone set by the maintainer. |
| 59 | + |
| 60 | +### Merging |
| 61 | + |
| 62 | +The pull request will be merged once it is approved by relevant maintainers (as |
| 63 | +described above) and has passed all necessary checks. This can be done by any |
| 64 | +core package maintainer. In practice this is sometimes the PR developer and |
| 65 | +sometimes the reviewer. |
| 66 | + |
| 67 | +## Astropy Coordinated & Infrastructure Packages |
| 68 | + |
| 69 | +The process of PR approval for coordinated and infrastructure packages should be |
| 70 | +the same as the process for the core package except that reviews and approvals |
| 71 | +can be done by any maintainer. Coordinated and infrastructure packages should |
| 72 | +have at least two maintainers with push access and all PRs should be approved by |
| 73 | +at least one of the other maintainers. Any maintainer can then merge the pull |
| 74 | +request once it is approved. |
| 75 | + |
| 76 | +## Astropy Affiliated Packages |
| 77 | + |
| 78 | +The change process for affiliated packages is largely at the discretion of the |
| 79 | +package maintainers. Affiliated packages may make their requirements as rigorous |
| 80 | +or as lenient as they like. However, the Astropy Project does require that |
| 81 | +affiliated packages maintain the code under version control on a publicly |
| 82 | +available site. |
| 83 | + |
| 84 | +In most cases packages are hosted on GitHub or GitLab and updated via pull |
| 85 | +requests, but this is not a requirement. Independent review is encouraged but is |
| 86 | +at the discretion of the package maintainer(s) as some packages may be largely |
| 87 | +developed by a single maintainer. Please see the affiliated package’s |
| 88 | +documentation for more information. |
0 commit comments