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

Submit to CRAN #2

Open
AngelFelizR opened this issue Dec 23, 2024 · 2 comments
Open

Submit to CRAN #2

AngelFelizR opened this issue Dec 23, 2024 · 2 comments
Assignees

Comments

@AngelFelizR
Copy link
Owner

Okay, let's analyze your package files to see if they're ready for CRAN. I will focus on the items that are commonly checked by CRAN.

General Observations:

  • Functionality: The package seems to do what it says and it provides 2 main functions get_bible_verse and find_bible_verse.
  • Testing: You've included test files using the testthat framework, which is great. They include mock responses.
  • Documentation: You have a README.md, which is good.

Specific Issues:

  1. README.md:

    • Install Instructions:

      • You have two installation instructions: from GitHub and from CRAN. When submitting, you should adjust your README to focus mainly on installing from CRAN since this will be your main way of installing once the package is accepted in CRAN. You could keep the GitHub installation for development purposes, but make CRAN the primary one.
      • The CRAN install instruction is in a code chunk, but it doesn't need to be: install.packages("biblegatewayr") is enough.
    • Examples:

      • The examples should avoid cat(), but rather print the output normally since cat adds extra output in the console and make the examples to be more difficult to read.
      • The examples in the Readme could be more concise. The output should be hidden, this is not really required, since the tests already checks for the expected behaviour.
      • You could add more versions to test, for example, English versions.
      • You have an example using NTV, that should be placed in \donttest{} since there are some locale considerations to take in to account.
    • Main Components:

      • This section is good to have, but could be more concise, there is no need to point to the help documentation, since it's assumed that the users will explore it with ?.
  2. DESCRIPTION File:

    • Title: "Access Bible Verses from BibleGateway" is descriptive and uses title case, which is good.
    • Description: The description is well-written. I suggest adding software and package names in single quotes, e.g., 'get_bible_verse', 'find_bible_verse'.
    • Authors@R:
      • Good job using person(), including roles (aut, cre, cph) and ORCID.
    • License: You've correctly declared MIT + file LICENSE.
    • Imports: You've correctly declared packages such as httr2 and rvest.
    • Suggests: You've correctly declared testthat (>= 3.0.0).
    • URL: You have a secondary URL to your package website, this is great.
    • BugReports: You have a link to the issues page in GitHub, that's great.
  3. R Code Files (find_bible_verse.R and get_bible_verse.R)

    • Roxygen Comments:
      • You used @examplesIf interactive(), that's good, but keep in mind that this can lead to inconsistencies when building the package documentation in different environments.
      • You are missing @returns in both functions.
      • You have correctly documented all the parameters with @param.
    • Function Logic:
      • Both functions use httr2 and rvest correctly, performing HTTP requests and parsing HTML content.
      • You have a good error management strategy using tryCatch.
      • In general, the functions have very good formatting and good comments.
      • In get_bible_verse function, the logic to split and format the output is quite complex, that could lead to unexpected behaviours, consider to simplify this.
  4. test Files:

    • You have successfully implemented tests in a separate directory.
    • You're using httr2::with_mocked_responses to mock responses, which is best practice.
    • The tests are comprehensive, testing for success, failure and error scenarios.
    • Test are well structured and with good organization.

Recommendations Before CRAN Submission:

  1. README.md:

    • Revise the install instructions, prioritize CRAN installation and simplify the instructions.
    • Remove cat from the examples.
    • Hide output of examples.
    • Add \donttest to NTV example, or any other example that is using a non-english version.
    • Add English version examples.
    • Simplify "Main components" section.
  2. DESCRIPTION File:

    • Put all software and package names in single quotes in the description section. For example, 'get_bible_verse', 'find_bible_verse'.
  3. R Code Files

    • Add @returns tag for every exported function in roxygen comments.
    • In get_bible_verse function, try to simplify the logic used to format the text output.
  4. Extra Checks:

    • Check for the noSuggests and check results using devtools::check(args = c('--no-manual', '--no-vignettes', '--no-build-vignettes')) as it could reveal hidden issues.
    • Check for url_check() and fix all urls if required.

Additional steps:

  • cran-comments.md: add this file, and specify that this is a new submission and that all the checks passed.
  • NEWS.md: add this file, explaining the new features and changes you did in this version.

Summary:

Your package is in very good shape for a first CRAN submission.
By addressing the points mentioned above, you will have an higher chance of being approved in the first try.

Remember to run devtools::check(remote = TRUE, manual = TRUE) and devtools::check_win_devel() before submitting.

If you have more questions or would like to review again, please do not hesitate to ask.

@AngelFelizR

This comment was marked as outdated.

@AngelFelizR

This comment was marked as outdated.

@AngelFelizR AngelFelizR self-assigned this Dec 23, 2024
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

No branches or pull requests

1 participant