Skip to content

Reviewing a Pull Request

Oscar Esteban edited this page Jul 6, 2018 · 4 revisions

For guidelines on how to review software, please read https://ropensci.org/blog/2016/03/28/software-review/

When reviewing a PR, members of the fMRIPrep team are encouraged to use the following template (which is totally inspired by the rOpenSci template, also used by JOSS:

## PR Review

*Please check off boxes as applicable, and elaborate in comments below.  Your review is not limited to these topics, as described in the reviewer guide*

- [ ] As the reviewer I confirm that there are no conflicts of interest for me to review this work.

Please check what applies in the following aspects of the PR:

#### Code documentation
- [ ] New functions and modules are documented following the guidelines.
- [ ] Existing code required amendments in documentation and has been updated.
- [ ] Sufficient inline comments ensure readability by other contributors in the long term.

#### Documentation site

- [ ] The modifications to *fMRIPrep* in this PR **do not require extra documentation**. This is a common situation when a bug fix that does not change the code base substantially is submitted.
- [ ] This PR requires documentation. The corresponding documentation will be submitted as an additional PR in the future.
- [ ] This PR requires extra documentation, and will be included within this PR. Please check the boxes that apply best:
  - [ ] An existing feature is substantially modified, changing the interface and/or logic of a module.
  - [ ] A new feature is being added, therefore full documentation of it will be included
  - [ ] This PR addresses an error in existing documentation.
- [ ] Yes, all expected sections of documentation were generated by the CI build, and look as expected

#### Tests
- [ ] The new functionalities in this PR are covered by the existing tests
- [ ] New test batteries have been included with this PR

#### Data
- [ ] This PR does not require any additional data to be uploaded to OSF.
- [ ] This PR requires additional data for tests.

#### Dependencies: niworkflows
- [ ] This PR does not require any update on `niworkflows`.
- [ ] `niworkflows` has correctly been pinned.

#### Dependencies: Nipype
- [ ] This PR does not require any update on `nipype`.
- [ ] `nipype` has correctly been pinned.

#### Dependencies: other
- [ ] This PR does not require any update on other dependencies.
- [ ] Other dependencies have correctly been pinned.

#### Reports generated within CI tests
- [ ] Yes, I have checked the reports of ds005 and they are not modified or the changes are as expected
- [ ] Yes, I have checked the reports of ds054 and they are not modified or the changes are as expected
- [ ] Yes, I have checked the reports of ds010 and they are not modified or the changes are as expected

### Review Comments (other than those left inline with the code)