Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update PR template to be more comprehensive and usable #231

Open
wants to merge 38 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
cfee09d
Copy checklists directly into pull_request_template.md
kelliemac Aug 22, 2024
86754f1
change master to main
kelliemac Aug 22, 2024
bd1966c
Update pull_request_template.md
kelliemac Aug 23, 2024
087ca04
Update pull_request_template.md
kelliemac Aug 23, 2024
89b66b7
switch order of code review and writing review
kelliemac Aug 23, 2024
f4157f3
add documentation section
kelliemac Aug 23, 2024
f2c4a0d
add note about when/who to complete checklist
kelliemac Aug 23, 2024
0038164
Update pull_request_template.md
kelliemac Aug 23, 2024
0f5f92a
Update pull_request_template.md
kelliemac Aug 23, 2024
3aee6ee
Update pull_request_template.md
kelliemac Aug 23, 2024
db9eaab
Update pull_request_template.md
kelliemac Aug 23, 2024
e28c044
add link to VISCtemplates and VISCfunctions repos
kelliemac Aug 23, 2024
05a020b
Update pull_request_template.md
kelliemac Aug 23, 2024
d891873
Update pull_request_template.md
kelliemac Aug 23, 2024
b8360d4
Update pull_request_template.md
kelliemac Aug 23, 2024
d21065b
Update pull_request_template.md
kelliemac Aug 23, 2024
644e43e
Update pull_request_template.md
kelliemac Aug 23, 2024
b352b5e
Update pull_request_template.md
kelliemac Aug 23, 2024
5d63659
Update pull_request_template.md
kelliemac Aug 23, 2024
6907c26
Update pull_request_template.md
kelliemac Aug 23, 2024
0c21358
Update pull_request_template.md
kelliemac Aug 23, 2024
56ad167
Update pull_request_template.md
kelliemac Aug 23, 2024
32e7528
Update pull_request_template.md
kelliemac Aug 23, 2024
340483a
Update NEWS.md
kelliemac Aug 23, 2024
4f3407f
fix type
kelliemac Aug 26, 2024
b938473
split two checklist items apart
kelliemac Aug 26, 2024
8173961
add to checklist searching for stray warnings and R output
kelliemac Aug 26, 2024
ab44468
add suppressed warnings to checklist
kelliemac Aug 26, 2024
2b03b99
change wording about inserting numeric values in results section
kelliemac Aug 26, 2024
1e3a54a
Merge branch 'develop' into update-pr-templates
kelliemac Aug 26, 2024
6f34984
Update pull_request_template.md
kelliemac Sep 4, 2024
dedebef
Update pull_request_template.md
kelliemac Sep 4, 2024
ef9aef9
Update pull_request_template.md
kelliemac Sep 4, 2024
291219c
Update pull_request_template.md
kelliemac Sep 4, 2024
5020c48
Update pull_request_template.md
kelliemac Sep 4, 2024
ad35218
add note about data package version
kelliemac Sep 5, 2024
9d7abef
Merge branch 'develop' into update-pr-templates
slager Oct 31, 2024
91dc8a7
Merge branch 'develop' into update-pr-templates
slager Oct 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Other improvements
* create_visc_project() now discards README.Rmd after knitting template to README.md (#223)
* Update PT report naming practices to the format VDCnnn_assay_PTreport_interim/final_(un)blinded.Rmd (#202)
* Add auxiliary files to template .gitignore (.aux, .toc, .lof, .lot, .out, cache files, and .smbdelete files) (#230)
* Update analysis repo PR template to be more comprehensive and useful (#231)
* Update names in template acknowledgements section (#234)

# VISCtemplates 1.3.2
Expand Down
159 changes: 148 additions & 11 deletions inst/templates/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,158 @@
<!-- Provide a general summary of your changes in the Title above. -->

## Description
<!-- Describe your changes in detail.

Give a short background on the report, outline important questions or details for the reviewer, and add links to any supporting documents (e.g., protocol presentation).
Make sure to provide a brief summary of your changes in the pull request title above.

Here, describe your changes in detail.
Give a short background on the report, outline important questions or details for the reviewer, and add links to any supporting documents (e.g., protocol presentation).
It's helpful to add links to the key files with the unique ID for the commit (a.k.a. the "SHA" or "hash").

-->
Make sure to include any known outstanding issues as well (and if they will be addressed in future PRs).

## Reflection

Describe any key challenges you faced in working on these changes. Were these challenges unique to this project, or do you think they apply to other VISC projects as well? If not unique, have any relevant GitHub issues in the [VISCtemplates](https://github.com/FredHutch/VISCtemplates) or [VISCfunctions](https://github.com/FredHutch/VISCfunctions) repos been created that would help for future projects?

## Checklist(s) for PR Creator

Use one (or multiple) of the following checklists, depending on which type of PR you are doing.

Note: checklists should be completed before a pull request is submitted for review. You can create a draft pull request before completing the checklist, then complete the checklist, and then mark the PR as ready for review.

### Documentation and completeness (use for all PRs)

- [ ] Necessary context for the project/analysis has been documented
- [ ] Appropriate README.md files have been updated to reflect the latest changes
- [ ] The latest versions of all relevant files have been pushed to the repo
- [ ] Unrelated or unnecessary files aren't included (e.g., LaTeX .toc files)

### Code review (use for PRs that include any code, such as in .R or .Rmd files)

See also [code review guidelines](https://github.com/FredHutch/VISC-Documentation/blob/main/Programming/code-review-guideline.md)

- [ ] I have compiled the R Markdown file(s) (or run the relevant code) with no errors
- [ ] Warnings are not suppressed. If a warning must be suppressed there is a clear explanation (i.e., comment).
- [ ] Running time has been recorded or estimated: ___________
- [ ] I have reviewed the data processing and statistical analysis code for logical correctness
kelliemac marked this conversation as resolved.
Show resolved Hide resolved
- [ ] I have double-checked any joins
- [ ] I have double-checked any filtering and it is in a logical order
- [ ] For PT reports: the analysis code follows and agrees with the statistical methods section
- [ ] Appropriate R packages are used
- [ ] VISCtemplates and VISCfunctions are used whenever possible
- [ ] No local package installations or unnescessary packages
- [ ] If renv is used, I have run `renv::status()` and resolved any issues
- [ ] Code is readable and easy to understand, and follows the [VISC Coding Principles](https://github.com/FredHutch/VISC-Documentation/blob/main/Programming/Coding-Principles.md) document
- [ ] Lines are at most 100 characters long
- [ ] Consistent use of `<-` not `=` for assignment
- [ ] Object names are meaningful, descriptive, and use only alphanumeric characters and underscores (no dots)
- [ ] Object names are unique (no overwriting of previous variables)
- [ ] Rmd code chunk names are descriptive and use dashes (not underscores or spaces)
- [ ] Functions are organized and well-documented, with explanations of purpose, inputs, and ouput
- [ ] Comments do not include unaddressed debt (e.g. `# TODO:` or `# FIXME`)
- [ ] Commented-out backup code and unused chunks have been removed
- [ ] File paths are relative (except for trials and network drive paths) and portable across operating systems (use `file.path()`)
- [ ] Hard coding and magic numbers are avoided

### Writing/report review (use for PRs with PDF and/or Word drafts of PT reports)

See also [writing review guidelines](https://github.com/FredHutch/VISC-Documentation/tree/main/Writing_Reviewing/writing_reviewing_guidelines.md)

- [ ] The latest version of the report has been compiled to both PDF and Word without errors
- [ ] I've opened and reviewed the compiled PDF document
- [ ] I've opened and reviewed the compiled Word document
- [ ] There are no obvious Markdown/pandoc/Latex errors
- [ ] No broken references (?? or ???) in the text (Use find: “??”)
- [ ] No stray warnings or R output in the text (Use find: “#”)
kelliemac marked this conversation as resolved.
Show resolved Hide resolved
- [ ] No blank pages
- [ ] Page x out of y is correct (sometimes y is wrong)
- [ ] The reproducibility tables look correct
- [ ] The reproducibility tables do not include `NA`, local installations, or unnescessary packages
- [ ] The most recent versions (note: not the development versions) of VISCtemplates and VISCfunctions are used
- [ ] The data package git hash refers to the correct branch/version (i.e., is up-to-date)
- [ ] The sample type is accurate (e.g., serum, plasma, PBMC)
- [ ] Text has been spell-checked (including captions and footnotes)
- [ ] The report text, including figure and table captions, follows VISC conventions (refer to the [writing guidelines](https://github.com/FredHutch/VISC-Documentation/tree/main/Writing_Reviewing) as needed)
- [ ] Objectives follow the SAP and study protocol
- [ ] Results and summary of main results sections map to the objectives
- [ ] Everything mentioned in the Summary of Main Results is also in the Results section
- [ ] The correct tense (generally past tense) is used throughout the report
- [ ] Capitalization is correct and consistent
- [ ] Acronyms and abbreviations are introduced the first time they are used
- [ ] I have reviewed the results sections carefully and confirmed that the statements in Results section are correct (including p-values) and supported by the correct figure and table references
- [ ] Code-based methods (i.e., in-line referencing) are used in inserting numeric values in the Results section (to minimize human error)
- [ ] I have reviewed the figures and tables carefully
- [ ] Figures and tables are sorted in parallel with mentions in Results section
- [ ] Figures look right (refer to the [figure guidelines](https://github.com/FredHutch/VISC-Documentation/blob/main/Programming/figure-guidelines.md) as needed)
- [ ] The appropriate number of axis tick marks is present (at least 3) for each figure
- [ ] Text is not cut off (facet labels, legends, titles)
- [ ] Tables look right
- [ ] Text is not running off the page
- [ ] Significance highlighting is as expected

## Notes

Add any additional notes here.

If necessary, provide explanations here for why any boxes from the checklist(s) above are not checked.

- Reason 1
- Reason 2

## Checklist(s) for PR reviewer(s)

Use one (or multiple) of the following checklists, depending on which type of PR this is.

### Documentation and completeness

## Checklist
- [ ] Necessary context for the project/analysis has been documented, and appropriate README.md files appear to be updated
- [ ] The latest versions of all relevant files appear to be pushed to the repo, and no unrelated or unnecessary files are included

<!-- Use one of the following checklists depending on which type of PR you are
doing. Copy and paste the checklist below.
### Code review

* Peer writing review: https://github.com/FredHutch/VISC-Documentation/tree/master/Writing_Reviewing
- [ ] If requested, I have compiled the R Markdown file(s) (or run the relevant code) on my end with no errors
- [ ] File paths are relative (except for trials and network drive paths) and portable across operating systems
- [ ] Warnings are not suppressed. If a warning must be suppressed there is a clear explanation (i.e., comment).
- [ ] There are no unused Rmd chunks or commented-out backup code
- [ ] Appropriate R packages are used (VISCtemplates and VISCfunctions are used where possible, no local package installations or apparently unused packages)
- [ ] Code appears logically correct
- [ ] I have reviewed any joins and they appear correct
- [ ] I have reviewed any filtering and it appears correct and in a logical order
- [ ] Code is readable and easy to understand, and generally follows the [VISC Coding Principles](https://github.com/FredHutch/VISC-Documentation/blob/main/Programming/Coding-Principles.md)
- [ ] Lines are not excessively long
- [ ] Assignment operator `<-` is used consistently (rather than `=`)
- [ ] Object names are meaningful, descriptive, and use only alphanumeric characters and underscores (no dots)
- [ ] Object names are unique (no overwriting of previous variables)
- [ ] Hard coding and magic numbers are avoided
- [ ] Rmd code chunk names are descriptive and use dashes (not underscores or spaces)
- [ ] Functions are organized and well-documented (with explanations of purpose, inputs, and ouput)
- [ ] Comments are helpful and do not include unaddressed debt (e.g. `# TODO:` or `# FIXME`)
- [ ] For PT reports: the analysis code follows the statistical methods section

* Code review: https://github.com/FredHutch/VISC-Documentation/blob/master/Programming/code-review-guideline.md
### Writing/report review

-->
- [ ] Both PDF and Word versions of the report are included and generally look acceptable
- [ ] There are no obvious Markdown/pandoc/Latex errors
- [ ] No broken references (?? or ???) in the text (Use find: “??”)
- [ ] No stray warnings or R output in the text (Use find: “#”)
- [ ] No blank pages
- [ ] Page x out of y is correct (sometimes y is wrong)
- [ ] The reproducibility tables look correct
- [ ] The reproducibility tables do not include `NA`, local installations, or unnescessary packages
- [ ] The most recent versions (note: not the development versions) of VISCtemplates and VISCfunctions are used
- [ ] The sample type is accurate (e.g., serum, plasma, PBMC)
- [ ] Objectives follow the SAP and study protocol
- [ ] Results and summary of main results sections map to the objectives
- [ ] No obvious spelling errors (including captions and footnotes)
- [ ] The correct tense (generally past tense) is used throughout the report
- [ ] Capitalization is correct and consistent
- [ ] Acronyms and abbreviations are introduced the first time they are used
- [ ] I have reviewed the results sections
- [ ] Everything mentioned in the Summary of Main Results is also in the Results section
- [ ] Statements in Results section are correct (including p-values) and supported by the correct figure and table references
- [ ] I have reviewed the figures and tables
- [ ] Figures and tables are sorted in parallel with mentions in Results section
- [ ] Figures generally look right (refer to the [figure guidelines](https://github.com/FredHutch/VISC-Documentation/blob/main/Programming/figure-guidelines.md) as needed)
- [ ] The appropriate number of axis tick marks is present (at least 3) for each figure
- [ ] Text is not cut off (facet labels, legends, titles)
- [ ] Tables generally look right
- [ ] Text is not running off the page
- [ ] Significance highlighting is as expected
Loading