ropensci-review-tools / pkgcheck

Check whether a package is ready for submission to rOpenSci's peer-review system
https://docs.ropensci.org/pkgcheck/
18 stars 6 forks source link

modify and (re-)order checks? #81

Closed maelle closed 2 years ago

maelle commented 3 years ago
mpadge commented 3 years ago

Quote from #84, in the context of differentiating inputs to check functions, or input versus output of check functions:

that would also require splitting the main object into two objects, one as input (everything but checks), and one as output (the checks). That division would be artificial, and they'd have to be stitched back together somewhere anyway. But more importantly: The current design intentionally allows a check to also insert additional checks$info if that needs to be pulled in from elsewhere. That's not done at present, but is possible. If checks can't see checks (which would be good in principle), they also couldn't be expected to modify info, and that would reduce extensibility in that context.

mpadge commented 2 years ago

@maelle Finally returning to this issue. Current answers all extend from the collate_checks() function:

https://github.com/ropensci-review-tools/pkgcheck/blob/684aa2eda5e34739e6534b1024b692732c53a3bf/R/pkgcheck-fn.R#L193-L208

Perhaps the most important aspect in this context is that checks are listed and therefore called alphabetically, with the exception that any extra checks in the global environment, or passed elsewhere via the extra_env parameter to pkgcheck(), are appended at the end. That means that these checks could indeed readily modify the internally hard-coded ones.

Technically yes, but only according to the order described above.

Same answer.

Not at all in current implementation. Do you have any particular use cases for doing so? Any further thoughts on how and why this might be useful? - And note the order in which checks appear in the summary output is completely independent of the alphabetical order used in collate_checks(), and is hard-coded here:

https://github.com/ropensci-review-tools/pkgcheck/blob/66ce809ffd36b5b5bad5b826bcc8a1304fb8d488/R/summarise-checks.R#L92-L117

That same order could be used in the initial collation, but is not at present. Thinking about this nevertheless led to a new test file which will fail if new checks are added yet not listed in the order_checks function. I'm really very happy with most aspects of how this package works, but admit that this hard-coded ordering function is a little clunky. Any brilliant insights into how to do it better would be really appreciated!

maelle commented 2 years ago

Thank you! Could this info go into the contributing guide? I think it's fine as is but good to document.

mpadge commented 2 years ago

@maelle that commit finishes the vignette, with examples of locally-defined checks, and links to where check ordering is defined. Thanks.