Closed kbvernon closed 23 hours ago
codcov is being a bit extreme here. We can address the coverage later on. It is actually penalizing this PR for adding more checks funnily enough.
Good work @kbvernon
Attention: Patch coverage is 52.80899%
with 84 lines
in your changes missing coverage. Please review.
Project coverage is 80.31%. Comparing base (
d8689d9
) to head (d46ee51
). Report is 1 commits behind head on main.
Files with missing lines | Patch % | Lines |
---|---|---|
R/license_note.R | 0.00% | 69 Missing :warning: |
R/run_cargo.R | 72.22% | 10 Missing :warning: |
R/clean.R | 88.46% | 3 Missing :warning: |
R/read_cargo_metadata.R | 94.44% | 1 Missing :warning: |
R/use_crate.R | 95.45% | 1 Missing :warning: |
🚨 Try these New Features:
This PR tries to bring consistency to how functions call
processx::run()
. The main goal is to unburden this package by lettingprocessx
do the heavy lifting wherever possible. Changes include:error_on_status = TRUE
everywhereecho
parameter and passing it toecho_cmd = echo
(print cargo command to console) andecho = echo
(print stdout to console). The default isTRUE
to ensure that maximum information is printed to the console.env = get_cargo_envvars()
. I'm not sure this one is strictly necessary. I believe the function was originally written to handle issues with rtools and windows, but functions that don't use this still pass CI on all OS.wd
. This is now done with a single call torprojroot::find_package_root_file()
.jsonlite::parse_json(simplifyDataFrame = TRUE)
whenever cargo command returns json formatted strings in stdout. If we can always assume data.frames rather than lists, I think this will simplify future maintenance.At the same time, I took the liberty of adding standalone type checks, reformatting to ensure scripts past lintr tests, removing magrittr pipes, and replacing redundant code.
Right now, there are seven files with calls to
processx::run()
, but I only updated four of them:I'm ignoring cran-compliance.R because I think Josiah's recent work will make that code obsolete. I think the myriad issues in
source.R
are substantial enough that they call for their own PR. And the call inutils.R
is only really there to check for third party cargo tools, so maybe not worth messing with.