-
Notifications
You must be signed in to change notification settings - Fork 2
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
Create best practise guidelines #2
Open
sgetalbo
wants to merge
5
commits into
reconhub:master
Choose a base branch
from
lockedata:ld_guides
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,81 @@ | ||||||||||||||||||
--- | ||||||||||||||||||
title: "Best Practice for R Package Creation" | ||||||||||||||||||
author: "Ellen Talbot - Locke Data" | ||||||||||||||||||
date: "`r format(Sys.time(), '%A %d %B %Y')`" | ||||||||||||||||||
output: | ||||||||||||||||||
html_document: | ||||||||||||||||||
code_folding: show | ||||||||||||||||||
highlight: pygments | ||||||||||||||||||
number_sections: yes | ||||||||||||||||||
theme: spacelab | ||||||||||||||||||
toc: yes | ||||||||||||||||||
toc_collapse: no | ||||||||||||||||||
toc_depth: 2 | ||||||||||||||||||
toc_float: yes | ||||||||||||||||||
css: !expr here::here('css', 'style.css') | ||||||||||||||||||
--- | ||||||||||||||||||
|
||||||||||||||||||
```{r setup, include = FALSE} | ||||||||||||||||||
knitr::opts_chunk$set(echo = TRUE, | ||||||||||||||||||
collapse = TRUE, | ||||||||||||||||||
fig.width = 8, | ||||||||||||||||||
fig.height = 6, | ||||||||||||||||||
dpi = 150, | ||||||||||||||||||
warning = FALSE, | ||||||||||||||||||
message = FALSE) | ||||||||||||||||||
``` | ||||||||||||||||||
|
||||||||||||||||||
This is a high-level checklist for RECON packages that is backed by the broader information included in [rOpenSci Packages: Development, Maintenance, and Peer Review](https://devguide.ropensci.org/) and [R Packages: Organize, Test, Document, and Share Your Code](http://r-pkgs.had.co.nz/). Fundamentally, we want to write packages that people can use to support their work and make an impact. Our "audience" includes people with different levels of R skill and time to learn how to use a package so most of our guidelines are about helping you thinking about how you can make your package robust, error free, and accessible. | ||||||||||||||||||
|
||||||||||||||||||
# Setup | ||||||||||||||||||
- We recommend that you setup your package using [{recontools}](https://github.com/reconhub/recontools) | ||||||||||||||||||
- You should ideally host on GitHub | ||||||||||||||||||
|
||||||||||||||||||
# Package contents | ||||||||||||||||||
## README | ||||||||||||||||||
- Ensure the use of non-RECON user friendly language, try not to refer to internal processes etc. | ||||||||||||||||||
- Include a clearly defined "happy path" workflow that includes enough code to demonstrate the basic workflow | ||||||||||||||||||
- Clearly state what problem your package is solving and why someone should use it | ||||||||||||||||||
+ Include an elevator pitch and examples of where and why the package has been used e.g. "You’re in DRC, have low internet connection, are working on X situation..." | ||||||||||||||||||
|
||||||||||||||||||
## Functions | ||||||||||||||||||
- Follow the coding rules in the `golden_rules` document | ||||||||||||||||||
- Functions should be clearly commented | ||||||||||||||||||
- Internal functions should be tagged as so | ||||||||||||||||||
- Commented out code should be removed to improve readability. | ||||||||||||||||||
- All functions both internal and external should have roxygen documentation | ||||||||||||||||||
+ At a minimum internal (non-exported) functions should include a purpose and a brif description e.g. `#' This function is designed to...` | ||||||||||||||||||
- Avoid using personal/colloquial language when writing comments and context - non-RECON users need to understand too. | ||||||||||||||||||
- Detailed examples for functions should appear in the roxygen comments rather than the README | ||||||||||||||||||
|
||||||||||||||||||
## Tests | ||||||||||||||||||
- Tests should ideally be written prior to the code | ||||||||||||||||||
+ This could be README driven development where you write the "happy path" first and expected results, or more robustly, applying [Test Driven Development](http://agiledata.org/essays/tdd.html) so that you write some key tests first and then develop the function to meet those requirements outlined in the tests | ||||||||||||||||||
- Basic tests should test defaults first and foremost | ||||||||||||||||||
- Test coverage should be as comprehensive as possible | ||||||||||||||||||
- {testthat} is the recommended package for writing unit tests | ||||||||||||||||||
- Use `testthat::skip()` rather than commenting our tests that are problematic | ||||||||||||||||||
|
||||||||||||||||||
## Quality checks | ||||||||||||||||||
Use packages like {devtools}, {lintr}, and {goodpractice} to ensure your code passes quality checks, correct spellings, and meets common formatting standards. | ||||||||||||||||||
|
||||||||||||||||||
```r | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool! :) |
||||||||||||||||||
devtools::check() | ||||||||||||||||||
goodpractice::gp() | ||||||||||||||||||
lintr::lint_package() | ||||||||||||||||||
devtools::spell_check() | ||||||||||||||||||
``` | ||||||||||||||||||
|
||||||||||||||||||
# Collaboration | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe a section on branch protection is missing, so I've taken the liberty to add it here:
Suggested change
|
||||||||||||||||||
## GitHub best practices | ||||||||||||||||||
- Our [guidelines for submitted packages to RECON](https://www.repidemicsconsortium.org/resources/guidelines/) include some great advice about how to set up an effective GitHub environment for your package | ||||||||||||||||||
|
||||||||||||||||||
## Contributing Guidelines | ||||||||||||||||||
- Contributing guidelines should be included in each package repo, there is a good example at `reportfactory/CONTRIBUTING.md`. | ||||||||||||||||||
|
||||||||||||||||||
## Issue and Pull Request Templates | ||||||||||||||||||
- Issue and PR templates should be included in each package repo, there is a good example at `reportfactory/.github`. | ||||||||||||||||||
|
||||||||||||||||||
**Guidelines and templates all improve the chances of you having a more manageable workload and a great relationship with helpful contributors!** | ||||||||||||||||||
|
||||||||||||||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For internal function, can you precise if you mean full roxygen doc (e.g. incl examples), or just
@param
?