Add a script for comparing lint output on branches/PRs #760
Conversation
There was a problem hiding this comment.
Thanks for adding this, mostly looks sound.
I hadn't heard of gert before, so thanks for that tip.
Would it be possible to allow the user to turn off running the load_all step; and could we use optparse for option handling, rather than rolling our own handler, please.
|
Didn't add the option for turning What I think should be optional is running Anyway I'll take a closer look at it later. |
|
Sorry about that. I confused myself. Please disregard my note about devtools::load_all; I mistakenly thought you were loading the analysed package prior to linting. |
|
The failing tests in CI aren't related to this PR. |
|
Running But I ran into very messy issues while running the tests for packages that are needed by the script and contain binaries (e.g. purrr, rlang, dplyr, tidyr, ...) so that feature should probably come with more safety measures like running in separate R processes and maybe installing the active lintr branch into a temporary local library. |
|
Also needed for |
* add name attribute to Linter class fixes #746 * fix test failures * document() * restore 100% coverage for utils.R * deprecate Lint(linter = ...) and remove all calling instances make expect_lint() resilient to complete removal of the argument * add NEWS bullet * document() * fix lint, collapse with space fix test expectation Co-authored-by: Michael Chirico <[email protected]>
skip Depends except on object_usage_linter typo need check higher up forgot to supply arg just exit early if Depends unavailable provide an interactive() experience for debugging tweak
|
Added some functionality for running interactively, I think it works pretty nicely. Would be happy to merge now, PTAL |
* add name attribute to Linter class fixes #746 * fix test failures * document() * restore 100% coverage for utils.R * deprecate Lint(linter = ...) and remove all calling instances make expect_lint() resilient to complete removal of the argument * add NEWS bullet * document() * fix lint, collapse with space fix test expectation Co-authored-by: Michael Chirico <[email protected]>
not working from command line testing more debugging Rscript sucks progress -- we need to skip missing Imports too more progress -- skip on platforms without tcl/tk need testing again need to exit early
MichaelChirico
force-pushed
the
add-dev-script
branch
from
8709774 to
ed416eb
Compare
Basically extending @russHyde / @AshesITR script here:
https://gist.github.com/russHyde/568fd5af558c860d97e1b932ca773ff7
And tracking it here for transparency/easy versioning. Put in .dev which can hold various helpers for doing dev on the package.
Have been testing it on some sample CRAN packages but it's tough to get started. Lots of Depends to install from packages chosen at random... library is bloating quickly 😸