Add suggested fixes for straightforward packaging issues

[Bisaloo]

Some issues are relatively straightforward and can often be fixed with a one-liner from usethis (e.g. 'DESCRIPTION' does not have a BugReports field.). I believe it would be useful to suggest possible fixes for these issues.

As someone conducting reviews, I often find myself repeating these things and I believe they could be part of the automation. I also believe it's slightly more empowering for submitters to be able to fix their issues on their own (with the help of the bot) rather than having to wait for input from the editor.

[mpadge]

Thanks @Bisaloo. I agree with you, but find it hard to conceive of a useful place where that kind of information might be placed. The package works around a bunch of "checks", each of which has (or may have) two methods: summary and print. These are converted into the outputs seen on screen (and on the rendered HTML reports). The summary methods have to remain as one-liners to enable easy visual perception of the whole state of the package in as little screen space as possible, so i don't think suggestions can go there.

That leaves the print method, which is organised into distint sections reporting on various aspects of package structure (dependencies and statistical properties) and functionality (goodpractice), concluding with an "miscellaneous checks" which may fail. I don't think including suggested fixes in print methods would be viable, because these are all intended to print information on the package itself, and suggested fixes are a different category of stuff.

I suspect that if "suggested fixes" were to be implemented, the best approach would be to add an additional suggest_fix "method" to each check, which would by default be empty. We could then append an additional section to the main print output, after the current four main ones. The reporting functions could then check whether any failing checks also have non-empty suggest_fix fields, and if so then create this additional section and dump the contents of the corresponding suggest_fix fields.

Do you have any other suggestions? @maelle I'd also like to hear your thoughts on this.

[maelle]

The end goal would be for these hints to appear in software-review threads, what method is that?

[mpadge]

"method" in the context of the technical internal "methods" of the package itself. Currently only has these two methods:

output_pkgchk_<name_of_check> <- function (checks) {

    out <- list (
                 check_pass = TRUE | FALSE,
                 summary = "<whatever>",
                 print = "<print method>"
    )
    return (out)
}

All checks follow that structure, and so have only print and summary methods; my suggestion above would add suggest _fix methods. The package report would then optionally include an additional section, "Suggested fixes" or something, which would contain the output of all those fields for which checks failed. One advantage of that would be the ability to define an additional command, @ropensci-review-bot suggest fixes, to dump just the suggested pacakge fixes. That might be kinda cool, and should be very easy to implement as a direct copy of check package but which sends call to a (slightly) different API endpoint (or same on with additional parameter).

[maelle]

Thank you!

Do you think the fixes would be as easy to find if they do not appear right beside the red crosses text?

[mpadge]

Do you think the fixes would be as easy to find if they do not appear right beside the red crosses text?

I think so, yes. The collapsible sections make the structure pretty clear. There are currently 4 main sections, and this would add an additional fifth section. I wouldn't like to think that sections might go on expanding indefinitely, but 5 still seems okay to me. What we then really need to do before heading off to address this issue is list here what check failures might or could be translating into suggested fixes? I'll start another comment that we can all edit and compile a list (from the output of pkgcheck::list_pkgchecks()).

[mpadge]

Suggested Fixes

check	Suggest fix?	Suggestion
ci_badges	:heavy_check_mark:	Link to CI chapter in Dev Guide
pkgchk_fns_have_exs	:heavy_multiplication_x:
pkgchk_has_bugs	:heavy_check_mark:	"Add BugReports field to DESCRIPTION with URL of GitHub issues page for package usethis::use_github_links()"
pkgchk_has_citation	:heavy_check_mark:	Link to Citation Chapter of Dev Guide usethis::use_citation()
pkgchk_has_codemeta	:heavy_check_mark:	Link to package metadata recommendations in Dev Guide codemeta::write_codemeta() (Maëlle: note this is codemeta not codemetar as it is lighter)
pkgchk_has_contrib_md	:heavy_check_mark:	Link to Contributing Guide in Collaboration chapter of Dev Guide usethis::use_tidy_contributing() + tweaks
pkgchk_has_scrap	:heavy_multiplication_x:
pkgchk_has_url	:heavy_check_mark:	"Add URL field to DESCRIPTION with URL of GitHub page for package" usethis::use_github_links()
pkgchk_has_vignette	:heavy_multiplication_x:	usethis::use_vignette(<pkgname>)
pkgchk_left_assign	:heavy_multiplication_x:
pkgchk_license	:heavy_multiplication_x:	Link to licence chapter of R packages book
pkgchk_obsolete_pkg_deps	:heavy_check_mark:	Link to Recommended Scaffolding section of Packaging Guide chapter of Dev Guide
pkgchk_on_cran	:heavy_multiplication_x:
pkgchk_pkgname_available	:heavy_multiplication_x:
pkgchk_renv_activated	:heavy_check_mark:	"Run renv::deactivate() and commit resultant changes"
pkgchk_srr	:heavy_multiplication_x:
pkgchk_unique_fn_names	:heavy_multiplication_x:
pkgchk_uses_roxygen2	:heavy_check_mark:	Link to Documentation section in Packaging Guide chapter of Dev Guide

[mpadge]

@maelle @Bisaloo Feel free to edit the above, add/remove/change as you see fit. Thank you both :smile:

[maelle]

Awesome, I made a few edits!

I don't think @Bisaloo can edit the issue text directly.