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

Add vignettes & update README #27

Merged
merged 135 commits into from
Mar 6, 2023
Merged

Add vignettes & update README #27

merged 135 commits into from
Mar 6, 2023

Conversation

jamesmbaazam
Copy link
Collaborator

@jamesmbaazam jamesmbaazam commented Jan 30, 2023

In this PR,

@jamesmbaazam jamesmbaazam added the enhancement New feature or request label Jan 30, 2023
@jamesmbaazam jamesmbaazam self-assigned this Jan 30, 2023
Copy link

@pratikunterwegs pratikunterwegs left a comment

Choose a reason for hiding this comment

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

Thanks for tagging me @jamesmbaazam, leaving some general comments here.

Overall looks okay - the vignettes build and the website renders for me - but the main step is to bring this branch up to date with main - this will need to be done with a rebase as for previous branches. Once that is done this branch will integrate CI checks and should give a better picture what more is needed. Adding a render-readme workflow might be useful. Locally, R CMD check fails for me with a couple of errors and notes. I've left some comments on the files; happy to take another look or help with the rebase.

  1. The 'introduction.Rmd' vignette seems to be a repeat of the Readme. Could some more information be added that would make this more useful/comprehensive/not the same?
  2. Check the website version of the documentation using pkgdown::build_site() to check that the articles render in the intended order. If you want to separate the 'bp_literature' article into a separate section that needs to be done in a pkgdown.yml file.
  3. Suggest a {styler} run (including on the vignettes, use style_dir()) and then a lint check as well. Also suggest using {goodpractice}.

R/data.R Outdated Show resolved Hide resolved
R/data.R Outdated Show resolved Hide resolved
R/simulate.r Outdated Show resolved Hide resolved
R/simulate.r Outdated Show resolved Hide resolved
R/simulate.r Outdated Show resolved Hide resolved
vignettes/projecting_incidence.Rmd Outdated Show resolved Hide resolved
vignettes/projecting_incidence.Rmd Outdated Show resolved Hide resolved
vignettes/projecting_incidence.Rmd Outdated Show resolved Hide resolved
vignettes/projecting_incidence.Rmd Outdated Show resolved Hide resolved
vignettes/projecting_incidence.Rmd Outdated Show resolved Hide resolved
@jamesmbaazam
Copy link
Collaborator Author

  1. The 'introduction.Rmd' vignette seems to be a repeat of the Readme.

It's the same because it was moved to the README and is now deleted.

@jamesmbaazam
Copy link
Collaborator Author

Thank you for your comments and suggestions. I have now implemented all the changes.

@jamesmbaazam jamesmbaazam merged commit 0d71505 into main Mar 6, 2023
@jamesmbaazam jamesmbaazam deleted the vignettes branch April 15, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove context() from tests Create pkgdown site for bpmodels Add use case vignettes
6 participants