Add lint CI and first pass at resolving issues #86
Conversation
There was a problem hiding this comment.
Looks good!! Thanks Eli for implementing - just some lintr trailing spaces to fix.
Also, shall we increase the cyclomatic complexity to 20 or try and fix?
This new action got me thinking that we should implement an action for Documentation so it stays up to date as the R-Cmd checks just re-build the package. This way the website stays current as well.
We have one in admiral I can bring over.
| @@ -39,7 +39,7 @@ xportr_format <- function(.df, metacore, domain = NULL, verbose = getOption("xpo | |||
| if (!is.null(attr(.df, "_xportr.df_arg_"))) df_arg <- attr(.df, "_xportr.df_arg_") | |||
There was a problem hiding this comment.
Shall we increase the cyclomatic complexity to 20 or try and figure this one out?
There was a problem hiding this comment.
Changing lines 76-84 to the code below should solve the problem.
The second loop is not really needed.
for (i in seq_len(ncol(.df))) {
format_sas <- purrr::pluck(format, colnames(.df)[i])
if (is.na(format_sas) || is.null(format_sas))
format_sas <- ""
attr(.df[[i]], "format.sas") <- format_sas
}from:
for (i in names(format)) {
attr(.df[[i]], "format.sas") <- format[[i]]
}
# Convert NA formats to "" for haven
for (i in seq_len(ncol(.df))) {
if (is.na(attr(.df[[i]], "format.sas")) || is.null(attr(.df[[i]], "format.sas")))
attr(.df[[i]], "format.sas") <- ""
}
There was a problem hiding this comment.
I suggest adding some project options in xportr.Rproj as part of this PR:
This will help ensure we enforce some linter rules automatically:
- Removing trailing whitespace when saving a file lint rule:
trailing_whitespace_linter - New lines at the bottom of file
trailing_blank_lines_linter
As well as avoid having native pipes when using the RStudio shortcut (Ctrl+Shift+M)
AutoAppendNewline: Yes
StripTrailingWhitespace: Yes
UseNativePipeOperator: No
* devel: Update metadata variable name in test Update NEWS.md Update tibble::tibble to dplyr::tibble to clear R CMD check warning Update `xportr_type()` to drop column attribute 'class' only Add test for label vs type order Update `xportr_type()` to retain column attributes
|
@elimillera I added 3 commits to the
|
Hey @bms63 here is the first go at linting, if something ends up to be too constraining, we can update that lintr. Just to make everyone aware, this will make other merges pretty nasty so I'm thinking we want to merge this in ASAP to make sure we don't have to duplicate work with merges.
I resolved most of the linting issues, there are some issues with comments in test and a function being overly complex.
closes #83