Skip to content

Commit

Permalink
(#186): few more edits
Browse files Browse the repository at this point in the history
  • Loading branch information
ccarouge committed May 15, 2024
1 parent 5d7b808 commit 8d73f90
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ To ask for a review, on the pull request screen, click on the Reviewers menu or

### Understand a review

Reviewers can either leave comments or suggestions during the review. All of these need to be resolved to finish the current review. The GitHub documentation has more information about reviews, in particular around [comments on PR][github-commentPR] and how to [incorporate feedback][github-feedback].
Reviewers can either leave comments or suggestions during the review. All of these need to be resolved to finish the current review. The GitHub documentation has more information about reviews, in particular around [comments on PR][github-commentPR] and how to [incorporate feedback][github-feedback].

#### Suggestions from the reviewer

Expand Down Expand Up @@ -123,6 +123,8 @@ Comments to a specific set of lines look like this:

It shows the specific lines commented on and the comment from the reviewer and it allows to reply to that specific comment.

Changes arising from these comments need to be applied on your local repository.

### Request a re-review

Once you have finished addressing all the comments from the reviewer, you should ask the reviewer to re-review the pull request:
Expand Down
33 changes: 21 additions & 12 deletions documentation/docs/developer_guide/contribution/review_guide.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Review guidelines

All contributions to the CABLE land surface model will be reviewed before inclusion in the model. The review process is intended to check that:
All contributions to the CABLE land surface model will be reviewed before inclusion in the model. The review is intended to ensure submissions to CABLE are of the best quality and integrate correctly within the CABLE's code design.

The review process checks that:

- [the coding standards][coding-standards] have been applied
- the documentation is understandable and follows at least the [minimum requirements][doc-min-req]
Expand All @@ -9,14 +11,20 @@ All contributions to the CABLE land surface model will be reviewed before inclus
- the proposed changes are correct
- the implementation of the changes follows the design of the CABLE model and will be maintainable

Once you have finished the implementation of your changes, please make sure:
## Final checks before asking for review

Once you have finished the implementation of your changes and before asking for a review, please make sure:

- the description of the pull request is up-to-date.
- the description of the pull request is up-to-date.
- all the required test results are either linked to in comments or copied in. If you have performed tests beyond the required tests, make sure these tests and their results are described in comments in your pull request.
- the automated checks pass. Contact the @CABLE-LSM/admin team if you need help understanding a failure of these tests.
- all conflicts have been solved.
- the automated checks pass. Contact the @CABLE-LSM/admin team, through a comment in your pull request, if you need help understanding a failure of these tests.
- all conflicts have been solved.

## Asking for review

Once you are ready, [ask for a review by `CABLE-LSM/reviewers`][how-ask-review]. If you want a specific individual, you can use the same process to choose that person.
Once you are ready, [ask for a review by `CABLE-LSM/reviewers`][how-ask-review]. If you want a specific individual, you can use the same process to choose that person. Using the reviewers team is preferred as it will spread the workload across reviewers.

In most cases, asking for one reviewer is enough. The system allows asking for multiple reviewers, please use this feature will parsimony. It is often easier to ask for a single reviewer first and add another reviewer later as necessary.

## Review process

Expand All @@ -28,11 +36,13 @@ The review is likely to be an iterative process between the reviewer and the aut

[To incorporate code changes requested by the reviewer][how-review], you often need to incorporate these changes to your local repository and push them to GitHub again. It is possible for the reviewer to suggest changes that can be apply directly in GitHub. We recommend to:

- first apply the suggestions you agree with via GitHub
- then update your local branch with `git pull`
- apply other changes required by the review locally to your branch
- push the fully revised version to GitHub (`git push`)
- and finally [ask for a re-review][how-re-review] once you have resolved all points raised by the reviewer
1. apply the suggestions you agree with via GitHub
1. update your local branch with `git pull`
1. apply other changes required by the review locally to your branch
1. push the fully revised version to GitHub (`git push`)
1. [ask for a re-review][how-re-review] once you have resolved all points raised by the reviewer

The reviewer might ask for a complementary review by another individual if they think they are not able to review all aspects of a submission.

To better understand the pull request interface for reviews on GitHub, please refer to [the GitHub documentation][github-review].

Expand All @@ -44,7 +54,6 @@ Once a reviewer has approved the pull request, you can merge it. We prefer if th
[doc-min-req]: ../documentation_guidelines/index.md
[tests-req]: testing.md
[how-ask-review]: resources/how_to.md#ask-a-review
[how-checks]: resources/how_to.md#
[how-review]: resources/how_to.md#understand-a-review
[how-re-review]: resources/how_to.md#request-a-re-review
[github-review]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests

0 comments on commit 8d73f90

Please sign in to comment.