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

Issue 12 add videos to readme #84

Merged
merged 8 commits into from
Aug 9, 2024
Merged

Conversation

ThomUK
Copy link
Collaborator

@ThomUK ThomUK commented Aug 8, 2024

closes #12
closes #59

Adds video links with thumbnails to the README (closing issue #12)
Adds basic use example to the README (closing issue #59)
Also changes the README to use R markdown to enable the example, and fix some formatting issues

Copy link
Collaborator

@matt-dray matt-dray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that the Rmd knits correctly. Links all appear to work, including the videos.

Approving, but I've added a few minor points as suggestions that you can batch-commit if you please; only one of them actually needs changing (an 'its' typo).

Might be possible to retain the Rmd only and have a GitHub Action to render it, rather than have to remember to re-knit to md after changes.


You can install the current version of {NHSRwaitinglist} from GitHub with:

```r
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```r
```{r}

README.Rmd Outdated
```r
# install.packages("remotes")
remotes::install_github("nhs-r-community/NHSRwaitinglist", build_vignettes = TRUE)

Copy link
Collaborator

@matt-dray matt-dray Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like pedantry, but I couldn't remember if blank lines in chunks end up being empty in the rendered version.

Suggested change

README.Rmd Outdated

## Installation

You can install the current version of {NHSRwaitinglist} from GitHub with:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add a reference here to needing a GitHub PAT (e.g. {usethis} guide or Happy Git With R).

README.Rmd Outdated
1. The [example walkthrough](https://nhs-r-community.github.io/NHSRwaitinglist/articles/example_walkthrough.html) vignette takes a step-by-step walk through the white paper, using the six core functions in this package.
2. The [three example waiting lists](https://nhs-r-community.github.io/NHSRwaitinglist/articles/three_example_waiting_lists.html) vignette simulates three closely related waiting lists, and uses package functions to explore some of their characteristics.

At it's most basic, the package can be used to simulate a waiting list (a dataframe of waiting list addition dates and removal dates), and then compute some important statistics. Of course, if you already have waiting list data ready to analyse, you can skip the simulation step...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
At it's most basic, the package can be used to simulate a waiting list (a dataframe of waiting list addition dates and removal dates), and then compute some important statistics. Of course, if you already have waiting list data ready to analyse, you can skip the simulation step...
At its most basic, the package can be used to simulate a waiting list (a dataframe of waiting list addition dates and removal dates), and then compute some important statistics. Of course, if you already have waiting list data ready to analyse, you can skip the simulation step in the code below.

README.Rmd Outdated

# review the waiting list statistics
knitr::kable(overall_stats)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

README.Rmd Outdated
This project is released with a Contributor [Code of Conduct](./CODE_OF_CONDUCT.md).
By contributing to this project, you agree to abide by its terms.

The simplest way to contribute is to raise an issue detailing the feature or functionality you would like to see added, or any unexpected behaviour or bugs you have experienced.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The simplest way to contribute is to raise an issue detailing the feature or functionality you would like to see added, or any unexpected behaviour or bugs you have experienced.
The simplest way to contribute is to [raise an issue](https://github.com/nhs-r-community/NHSRwaitinglist/issues) detailing the feature or functionality you would like to see added, or any unexpected behaviour or bugs you have experienced.

@ThomUK
Copy link
Collaborator Author

ThomUK commented Aug 9, 2024

Many thanks @matt-dray for the great feedback! All incorporated, thank you.

@ThomUK ThomUK merged commit 861cf52 into main Aug 9, 2024
8 checks passed
@ThomUK ThomUK deleted the issue_12_add_videos_to_readme branch August 9, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

README examples of packages use Add a link to the video overview to the README
2 participants