Introduction
This document is intended to be guidance for creators and reviewers of pull requests (PRs) in the admiral package. PR authors will benefit from shorter review times by closely following the guidance provided here.
A pull request into the devel
branch signifies that an
issue that has been “addressed”. This issue might be a bug, a feature
request or a documentation update. For transparency, we keep the issue
open until the devel
branch is merged into the
main
branch, which usually coincides with a release of
admiral to CRAN. This ensures that repeat issues are not
raised and if they are raised are quickly marked as duplicates and
closed.
Closely following the below guidance will ensure that our all our
“addressed” issues auto-close once we merge devel
into
main
.
Review Criteria
For a pull request to be merged into devel
it needs to
pass the automated R CMD check
, lintr
, and
task-list-completed
workflows on GitHub at a minimum. The
first two checks can be run locally using the
devtools::check()
and lintr::lint_package()
commands and are recommended to be done before pushing to GitHub. The
task-list-completed
workflow is exclusive to GitHub and
will be discussed later. In addition, the PR creator and reviewer should
make sure that
the Programming Strategy and Development Process are followed
the function is ADaM IG compliant
the function does what is intended for (as described in the header and corresponding issue)
the function header properly explains the intention of the function, the expected inputs (incl. permitted values of parameters) and the output produced; after reading the documentation the reader should be able to predict the output of the function without having to read the source code
the function has an accompanying set of unit tests; for derivations these unit test should have a code coverage of at least 90%; the whole package should have a coverage of >= 80%
the implemented derivation is in the scope of admiral, e.g. does not expect company specific input or hard-code company-specific rules
meaningful error or warning messages are issued if the input is invalid
documentation is created/updated by running
devtools::document()
functions which are supposed to be exported are listed in the
NAMESPACE
file; this requires an@export
tag in the function headerexamples print relevant source variables and newly created variables and/or records in their output
the
NEWS.md
file is updated with an entry that explains the new features or changesthe author of a function is listed in the
DESCRIPTION
fileall files affected by the implemented changes, e.g. vignettes and templates, are updated
So much Red Tape!
The admiral development team is aware and sympathetic
to the great many checks, processes and documents needed to work through
in order to do a compliant Pull Request. The
task-list-completed
GitHub workflow was created to help
reduce this burden on contributors by providing a standardized checklist
that compiles information from the Pull Request Review Guidance, Programming Strategy and Development Process vignettes.
The next three sections give a high-level overview of what a
contributor faces in opening a PR, and how a contributor interacts with
the task-list-completed
workflow in their PR.
Open a Pull Request
When a contributor opens a PR a lengthy standard text will be
inserted into the comment section. Please do not alter any of the
automated text. You will need to manually add
Closes #<insert_issue_number>
into the title of the
Pull Request. You can use the Edit button in the top right if you forget
this step at the start of your Pull Request. Besides that you are free
to add in additional textual information, screenshots, etc. at the
bottom of the automated text if needed to clarify or contribute to the
discussion around your PR.
Create a Pull Request
After you click the green Create pull request
button the
automated text that was inserted will be turned into a checklist in your
Pull Request. Each check box has been drawn from the previously
mentioned vignettes and presented in the recommended sequence. These
check boxes are meant to be a helpful aid in ensuring that you have
created a compliant Pull Request.
Complete the Pull Request checklist
The check boxes are linked to the task-list-completed
workflow. You need to check off each box in acknowledgment that you have
done you due diligence in creating a compliant Pull Request. GitHub will
refresh the Pull Request and trigger task-list-completed
workflow that you have completed the task. The PR can not be merged into
devel
until the contributor has checked off each of the
check box items.
Please don’t hesitate to reach out to the admiral team on Slack or through the GitHub Issues tracker if you think this checklist needs to be amended or further clarity is needed on a check box item.
GitHub Actions/Workflows
The task-list-completed
workflow is one of the several
workflows/actions used within admiral. These workflows
live in the .github/workflows
folder and it is important to
understand their use and how to remedy if the workflow fails. Workflows
defined here are responsible for assuring high package quality standards
without compromising performance, security, or reproducibility.
A synopsis of admiral’s workflows
Most workflows have a BEGIN boilerplate steps
and
END boilerplate steps
section within them which define some
standard steps required for installing system dependencies, R version
and R packages which serve as dependencies for the package.
The underlying mechanisms for installing R and Pandoc are defined in
r-lib/actions
, while the installation of system
dependencies and R package dependencies is managed via the Staged
Dependencies GitHub Action]. The latter is used in conjunction with the
staged_dependencies.yaml
file in order to install
dependencies that are in the same stage of development as the
current package.
Following the installation of system dependencies, R, and package dependencies, each workflow checks the integrity of a specific component of the admiral codebase.
check-templates.yml
This workflow checks for issues within template scripts. For example, in the admiral package there are several template scripts with admiral-based functions showing how to build certain ADaM datasets. As we update the admiral functions, we want to make sure these template scripts execute appropriately. Functions in the template scripts that are deprecated or used inappropriately will cause this workflow to fail. Click on the details button on a failing action provides information on the where the template is failing.
code-coverage.yml
This workflow measures code coverage for unit tests and reports the code coverage as a percentage of the total number of lines covered by unit tests vs. the total number of lines in the codebase.
The covr R package is used to calculate the coverage.
Report summaries and badges for coverage are generated using a series of other GitHub Actions.
links.yml
This workflow checks whether URLs embedded in code and documentation
are valid. Invalid URLs results in workflow failures. This workflow uses
lychee
to detect broken links. Occasionally this check will
detect false positives of urls that look like urls. To remedy, please
add this false positive to the .lycheeignore
file.
lintr.yml
Static code analysis is performed by this workflow, which in turn
uses the lintr R package. The .lintr
configurations in the repository will be by this workflow.
man-pages.yml
This workflow checks if the manual pages in the man/
directory of the package are up-to-date with ROxygen comments in the
code.
Workflow failures indicate that the manual pages are not up-to-date with ROxygen comments, and corrective actions are provided in the workflow log.
pkgdown.yml
Documentation for the R package is generated via this workflow. This
workflow uses the pkgdown framework to generate
documentation in HTML, and the HTML pages are deployed to the
gh-pages
branch.
Moreover, an additional Versions
dropdown is generated
via the multi-version-docs
GitHub Action, so that an end
user can view multiple versions of the documentation for the
package.
r-cmd-check.yml
This workflow performs R CMD check
for the package.
Failed workflows are typically indicative of problems encountered during
the check, and therefore an indication that the package does not meet
quality standards.
r-pkg-validation.yml
When a new release of the package is made, this workflow executes to
create a validation report via validation
action. The PDF
report is then attached to the release within GitHub.
readme-render.yml
If your codebase uses a README.Rmd
file, then this
workflow will automatically render a README.md
and commit
it to your branch.
spellcheck.yml
Spellchecks are performed by this workflow, and the
spelling R package is used to detect spelling mistakes.
Failed workflows typically indicate misspelled words. In the
inst/WORDLIST
file, you can add words and or acronyms that
you want the spell check to ignore, for example occds is not an English
word but a common acronym used within Pharma. The workflow will flag
this until a user adds it to the inst/WORDLIST
.
Common R CMD Check Issues
R CMD check
is a command line tool that checks R
packages against a standard set of criteria. For a pull request to pass
the check must not issue any notes, warnings or errors. Below is a list
of common issues and how to resolve them.
Check Fails Only on One Version
If the R CMD check
workflow fails only on one or two R
versions it can be helpful to reproduce the testing environment
locally.
To reproduce a particular R version environment open the
admiral project in the corresponding R version, comment
the line source("renv/activate.R")
in the
.Rprofile
file, restart the R session and then run the
following commands in the R console.
Sys.setenv(R_REMOTES_NO_ERRORS_FROM_WARNINGS = "true")
if (!dir.exists(".library")) {
dir.create(".library")
}
base_recommended_pkgs <- row.names(installed.packages(priority = "high"))
for (pkg in base_recommended_pkgs) {
path <- file.path(.Library, pkg)
cmd <- sprintf("cp -r %s .library", path)
system(cmd)
}
assign(".lib.loc", ".library", envir = environment(.libPaths))
r_version <- getRversion()
if (grepl("^4.1", r_version)) {
options(repos = "https://packagemanager.posit.co/cran/2021-05-03/")
} else if (grepl("^4.2", r_version)) {
options(repos = "https://packagemanager.posit.co/cran/2022-01-03/")
} else if (grepl("^4.3", r_version)) {
options(repos = "https://packagemanager.posit.co/cran/2023-04-20/")
} else {
options(repos = "https://cran.rstudio.com")
}
if (!requireNamespace("remotes", quietly = TRUE)) {
install.packages("remotes")
}
remotes::install_deps(dependencies = TRUE)
remotes::install_github("pharmaverse/admiral.test", ref = "devel")
remotes::install_github("pharmaverse/admiraldev", ref = "devel")
rcmdcheck::rcmdcheck()
This will ensure that the exact package versions we use in the
workflow are installed into the hidden folder .library
.
That way your existing R packages are not overwritten.
Package Dependencies
> checking package dependencies ... ERROR
Namespace dependency not required: ‘pkg’
Add pkg
to the Imports
or
Suggests
field in the DESCRIPTION
file. In
general, dependencies should be listed in the Imports
field. However, if a package is only used inside vignettes or unit tests
it should be listed in Suggests
because all
admiral functions would work without these “soft”
dependencies being installed.
Global Variables
❯ checking R code for possible problems ... NOTE
function_xyz: no visible binding for global variable ‘some_var’
Add some_var
to the list of “global” variables in
R/globals.R
.
Undocumented Function Parameter
❯ checking Rd \usage sections ... WARNING
Undocumented arguments in documentation object 'function_xyz'
‘some_param’
Add an @param some_param
section in the header of
function_xyz()
and run devtools::document()
afterwards.
Outdated Documentation
❯ checking for code/documentation mismatches ... WARNING
Codoc mismatches from documentation object 'function_xyz':
...
Argument names in code not in docs:
new_param_name
Argument names in docs not in code:
old_param_name
Mismatches in argument names:
Position: 6 Code: new_param_name Docs: old_param_name
The name of a parameter has been changed in the function code but not
yet in the header. Change @param old_param_name
to
@param new_param_name
and run
devtools::document()
.