Skip to content

Commit 701ca99

Browse files
taldcroftpllim
andauthored
Apply suggestions from code review
Co-authored-by: P. L. Lim <[email protected]>
1 parent ee0cd1d commit 701ca99

File tree

1 file changed

+14
-9
lines changed

1 file changed

+14
-9
lines changed

policies/pull-request-review-and-merge.md

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,34 @@ In the astropy core package the requirements are as follows:
99
- New or updated code is logically correct and achieves the desired effect of
1010
either fixing a bug or making an enhancement.
1111
- New tests or modifications to tests are adequate to cover the code changes.
12+
Tests for edge cases exist, if applicable.
1213
- There are no API changes or the API changes are well-understood and
1314
acceptable and beneficial on the whole to the astropy user community.
1415
- The expected level of review is detailed, including review and suggestions
1516
at the line-by-line level. Formatting and style are acceptable points for
16-
comment, in particular to maintain the existing style of a package and to
17+
comment, in particular to maintain the existing coding style of a package and to
1718
maintain readability of the code base for future developers.
18-
- A maintainer of sub-packages that the contribution affects is required to
19+
- Maintainer(s) of subpackages that the contribution affects is required to
1920
approve the pull request for changes that require specific knowledge of a
20-
sub-package. Other less complex cases, for example a documentation
21+
subpackage. Other less complex cases, for example a documentation
2122
improvement, may be reviewed by any astropy core package maintainer.
2223
- While a maintainer does not have to be the one doing the detailed review, if
2324
the detailed review is by someone else, the maintainer is responsible for
2425
ensuring the contributor review is sufficiently detailed and follows these
2526
guidelines.
26-
- After successful review, maintainers should formally approve the pull
27+
- Following a successful review, maintainers should formally approve the pull
2728
request.
28-
- A release manager or other maintainer checks that the selected release
29+
- A release manager or maintainer checks that the selected release
2930
milestone is suitable for the scope of the change. In particular bug-fix
3031
backports are considered where feasible and sensible.
32+
Documentation is usually backported. API change or new feature
33+
are not backported unless under special circumstances.
3134
- A matrix of CI checks ensure that all existing tests pass on supported
32-
platforms.
33-
- A bot checks that the change log mentions the change and that the changelog
34-
section used matches the selected release milestone.
35+
platforms. Even the CI job that is allowed to fail should pass unless
36+
an known and unrelated failure occurs; i.e., look at the logs even in
37+
the event of green check mark.
38+
- A bot checks that the change log mentions the change and PR number and that the change log
39+
section used matches the selected release milestone set by the maintainer.
3540
- Merging the pull request can be done by any core package maintainer once
3641
approved by relevant maintainers (as described above). In practice this is
3742
sometimes the PR developer and sometimes the reviewer.
@@ -40,7 +45,7 @@ In the astropy core package the requirements are as follows:
4045

4146
The process should in general be similar to the core package, although since a
4247
number of coordinated and infrastructure packages have fewer maintainers, the
43-
concept of sub-packages is not relevant. Nevertheless, all coordinated and
48+
concept of subpackages is not relevant. Nevertheless, all coordinated and
4449
infrastructure packages should have at least two maintainers with push access.
4550
All changes to these packages should be done via pull requests, and approved by
4651
at least one of the other maintainers (any maintainer can then merge the pull

0 commit comments

Comments
 (0)