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

Full package review for v0.4.0 release #164

Closed
wants to merge 566 commits into from
Closed

Full package review for v0.4.0 release #164

wants to merge 566 commits into from

Conversation

joshwlambert
Copy link
Member

This PR is to provide a platform to review the entirety of the package.

Once this review concludes I will release v0.4.0 on GitHub and submit to CRAN. This release will occur after {epiparameter} is accepted by CRAN.

Please see the NEWS.md file for an overview of changes between v0.3.0 and v0.4.0. If you would prefer to review with a partial package review only showing the changes between v0.3.0 and v0.4.0 please let me know and I can open one.

This PR is unconventional as it is not intended for merging or for additional commits (unless minor) and instead comments will be converted to issues and these will be addressed in their own PRs.

joshwlambert and others added 30 commits February 21, 2024 14:24
@joshwlambert joshwlambert added the pkg review Full package review label Oct 18, 2024
Copy link

This pull request:

  • Adds 49 new dependencies (direct and indirect)
  • Adds 6 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@jamesmbaazam jamesmbaazam self-requested a review October 18, 2024 10:10
Copy link
Member

@chartgerink chartgerink left a comment

Choose a reason for hiding this comment

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

Thanks @joshwlambert for being so active in inviting reviews in a timely fashion 😊

  1. I am pleased to see that you took my feedback from a previous moment on other packages, and made it possible to easily run these functions without having to specify further defaults. Being able to get started by running sim_linelist() is fairly rewarding 😄
  2. Your dilligent use of the NEWS.md and referring to the relevant issues is also 👍
  3. I have very little to mention on the testing here, even though I have been a broken record elsewhere. These tests are thorough and well done.

My only remaining question is: Do you want to release this on CRAN or is there still work to do to achieve the scope you are looking to achieve? It seems pretty solid to me, but I might be missing something you do see.


I am very glad to approve this. Moments like these are worth celebrating, so thank you @joshwlambert for delivering another good release! 🙌 🎊

@joshwlambert
Copy link
Member Author

Thanks for the review @chartgerink!

My only remaining question is: Do you want to release this on CRAN or is there still work to do to achieve the scope you are looking to achieve? It seems pretty solid to me, but I might be missing something you do see.

{simulist} is now ready to be submitted to CRAN. Once {epiparameter} is accepted onto CRAN, I will start the submission of {simulist} which will close #100.

@joshwlambert joshwlambert deleted the review branch January 8, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg review Full package review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants