Reviewing tests
Once someone has prepared one or more tests for functions in a package and submitted them to be incorporated into the platform via a pull request. They should then be reviewed by another member of the platform, ideally from another unit, to check that the tests are programmed and documented appropriately.
Abbreviations/Definitions
| Term | Definition |
|---|---|
| CTU | Clinical trial unit |
| RA | Reviewer assigner: member of the R Validation Steering Committee assigning the reviews to the CTUs |
| Contact person | One person per CTU responsible for the reviewer assignment among their CTU |
| Reviewer list | Randomized sequence of CTUs, weighted by the number of Statistics Platform members affiliated with each CTU |
| Reviewer | Person performing the review |
Who reviews?
Reviewer assignment process
The reviewer assigner (RA) maintains a reviewer list, a weighted randomized sequence of CTUs.
The RA is signed up as ‘observer’ on the validation tests GitHub repository and received a notification every time a Pull Request is made.
The RA then contacts the contact person of the next CTU on the reviewer list, asking them to assign a reviewer (among their team/CTU) to review the pull request with a deadline of one month.
If the contact person replies stating that they decline to assign a reviewer or the package comes from their CTU, the next CTU is contacted. The CTU which was skipped is set to the next on the list.
The contact person replies with the name of the reviewer and their contact details.
The RA indicates on the reviewer list (stored in Sharepoint) and on the pull request that the reviewer has been contacted.
After three weeks, the RA sends a reminder to the reviewer with the contact person in CC.
If needed, the RA sends another email after the deadline has passed.
When the task is finished, the RA indicates it on the reviewer list and on the pull request.
If you need a review urgently, reach out to someone by other means (e.g. email) and arrange that they perform the review for you.
Performing the review
The review is performed within the pull request on GitHub. There are four main tabs the pull request screen on GitHub:
- The conversation tab is for discussions of general points about the pull request.
- The commits tab lists the individual commits that make up the pull request. For our purposes, this is rarely of use.
- The checks tab shows the results of automated checks that are run on the pull request. As we have no automated checks running for this repository, this tab is also not useful.
- The files changed tab shows the changes that have been made in the pull request. This is where the review is performed.
The files changes lists all changes in all files modified during the pull request. The reviewer should look at each file in turn, checking that the changes are appropriate and that the code is well written and documented. The reviewer should also check that the tests are appropriate and that they test the correct things. Also, ensure that the details of the test in info.txt match the tests that were actually performed.
Where there are general points to be made, these should be made in the conversation tab. For specific points, the reviewer can comment on the specific line of code in the files tab.
- hover over the line to be commented on
- click on the
+that appears - type the comment in the box that appears
- click on
Start a reviewto submit the comment
To indicate that you have checked a particular file and that it is suitable, you can click on the Viewed box on the top right of each file.
The very top of the page has a box for finalising the review.
If there are no issues, the review can be marked with approve. If you have suggestions or require modifications, you can mark it as request changes. If you have questions, you can mark it as comment or request changes, whichever is most appropriate, and enter your questions in the box.
Incorporating the tests in the repository
Once the tests have been reviewed and found to be suitable and appropriately documented, the pull request needs to be merged into the repository. Each CTU has at least one nominated individual that can perform a merge (typically the same individuals distributing reviews). This individual should check that the review has been approved and then merge the pull request using the green button at the bottom of the conversation tab, followed by the “confirm merge” button that appears afterwards.