Advice for pre-commit.ci debugging

[lorenzwalthert]

There are a few issues where users might get lost or are unexpected. A good example is https://results.pre-commit.ci/run/github/146200962/1638570757.qIlnbiX1QlaCkdfzypQpaA.

lintr hook:

#> Error in loadNamespace(name) : there is no package called ‘checkmate’

Not sure what the problem is here. But we should probably handle packageNotFoundError more generally and tell people that they need to add the dependencies to additional_dependencies: It's already handled in the style-files hook and in the roxygenize hook but this should be more generic functionality.

# both give same error
tryCatch(xxerwer::df, error = function(e) class(e))
tryCatch(library(fjkjsf), error = function(e) class(e))

roxygenize hook

• ~Warning about cache. Maybe it should be disabled when the env variable CI is set (or narrow it down to pre-commit.ci), since these caches are mostly useful for local workflows.~ ✅ with {R.cache} > 0.15, it becomes obsolete -> remove it.
• the additional_dependencies: makes only sense if one does not hit the timeout. So the timeout should be mentioned. ✅
• the dev version of precommit is no longer needed for generating the roxygen snippet. ✅
• mention how one can skip the roxygenize hook on ci. ✅
• move all that documentation out of the error message into the ci vignette.

deps-in-desc

{knitr} is often used in README.md but it's not really a dependency (unless vignettes exist, then it's a suggests) since README.Rmd is in .Rbuildignore. I think this is a false positive and ~{knitr} should be excluded from README.Rmd when running these checks.~ README.Rmd should be excluded form the check. ✅

[lorenzwalthert]

@pat-s I'd welcome your comments.

[pat-s]

Some observations:

Warning about cache. Maybe it should be disabled when the env variable CI is set (or narrow it down to pre-commit.ci), since these caches are mostly useful for local workflows.

Given that there won't be a cache on CI, I think this would be good.

mention how one can skip the roxygenize hook on ci.

Maybe it should be disabled by default given the timeout issues and dependency installation issues?

move all that documentation out of the error message into the ci vignette.

How about adding a dedicated "precommit CI (troubleshooting)" vignette? And eventually link to it in failure outputs within precommit CI runs? (I guess there might be an env var which let's you detect precommit CI runs?)

More thoughts of mine on trying out precommit ci in my oddsratio package in #338

[lorenzwalthert]

Also: Lorenz Walthert think how to run all files through all hooks locally and when things pass, push.

[lorenzwalthert]

@pat-s I just released a new hook version (i.e. autoupdate() will be sufficient): https://github.com/lorenzwalthert/precommit/releases/tag/v0.2.0.9001 where I incorporated some of the ideas from our discussion.