Skip to content

Commit 97037fe

Browse files
typo in optparse funciton
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
1 parent 9f45127 commit 97037fe

File tree

1 file changed

+47
-13
lines changed

1 file changed

+47
-13
lines changed

.dev/compare_branches.R

+47-13
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,39 @@ param_list <- list(
2222
),
2323
optparse::make_option(
2424
"--branch",
25+
default = if (interactive()) {
26+
readline("Name a branch to compare to master (or skip to enter a PR#): ")
27+
},
2528
help = "Run the comparison for master vs. this branch"
2629
),
2730
optparse::make_option(
2831
"--pr",
32+
default = if (interactive()) {
33+
as.integer(readline("Name a PR # to compare to master (skip if you've entered a branch): "))
34+
},
2935
type = "integer",
3036
help = "Run the comparison for master vs. this PR"
3137
),
3238
optparse::make_option(
3339
"--packages",
34-
help = "Run the comparison using these packages (comma-separated)"
40+
default = if (interactive()) {
41+
readline("Provide a comma-separated list of packages (skip to provide a directory): ")
42+
},
43+
help = "Run the comparison using these packages (comma-separated)"
3544
),
3645
optparse::make_option(
3746
"--pkg_dir",
47+
default = if (interactive()) {
48+
readline("Provide a directory where to select packages (skip if already provided as a list): ")
49+
},
3850
help = "Run the comparison using all packages in this directory"
3951
),
4052
optparse::make_option(
4153
"--sample_size",
4254
type = "integer",
55+
default = if (interactive()) {
56+
as.integer(readline("Enter the number of packages to include (skip to include all): "))
57+
},
4358
help = "Select a sample of this number of packages from 'packages' or 'pkg_dir'"
4459
),
4560
optparse::make_option(
@@ -49,7 +64,13 @@ param_list <- list(
4964
)
5065
)
5166

52-
params <- optparse::parse_args(optparse::OptionParse(option_list = param_list))
67+
params <- optparse::parse_args(optparse::OptionParser(option_list = param_list))
68+
# treat any skipped arguments from the prompt as missing
69+
if (interactive()) {
70+
for (opt in c("branch", "pr", "packages", "pkg_dir", "sample_size")) {
71+
if (params[[opt]] == "") params[[opt]] = NULL
72+
}
73+
}
5374

5475
linter_names <- strsplit(params$linters, ",", fixed = TRUE)[[1L]]
5576

@@ -97,7 +118,7 @@ get_deps <- function(pkg) {
97118
deps
98119
}
99120

100-
lint_all_packages <- function(pkgs, linter) {
121+
lint_all_packages <- function(pkgs, linter, check_depends) {
101122
pkg_is_dir <- file.info(pkgs)$isdir
102123
pkg_names <- dplyr::if_else(
103124
pkg_is_dir,
@@ -116,11 +137,22 @@ lint_all_packages <- function(pkgs, linter) {
116137
utils::untar(pkgs[ii], exdir = tmp, extras="--strip-components=1")
117138
pkg <- tmp
118139
}
119-
# devtools::load_all() may not work for packages with Depends
120-
tryCatch(
121-
find.package(get_deps(pkg)),
122-
warning = function(w) stop("Package dependencies missing:\n", w$message)
123-
)
140+
# object_usage_linter requires running package code, which may
141+
# not work if the package has unavailable Depends
142+
if (check_depends) {
143+
try_deps <- tryCatch(
144+
find.package(get_deps(pkg)),
145+
error = identity, warning = identity
146+
)
147+
if (inherits(e, c("warning", "error")) {
148+
warning(sprintf(
149+
"Some package Dependencies for %s were unavailable: %s; skipping",
150+
pkg_names[ii],
151+
gsub("there (?:are no packages|is no package) called ", "", e$message)
152+
))
153+
return(NULL)
154+
}
155+
}
124156
lint_dir(pkg, linters = linter, parse_settings = FALSE)
125157
}
126158
) %>%
@@ -129,12 +161,12 @@ lint_all_packages <- function(pkgs, linter) {
129161

130162
format_lints <- function(x) {
131163
x %>%
132-
purrr::map(as_tibble) %>%
164+
purrr::map(tibble::as_tibble) %>%
133165
dplyr::bind_rows(.id = "package")
134166
}
135167

136-
run_lints <- function(pkgs, linter) {
137-
format_lints(lint_all_packages(pkgs, linter))
168+
run_lints <- function(pkgs, linter, check_depends) {
169+
format_lints(lint_all_packages(pkgs, linter, check_depends))
138170
}
139171

140172
run_on <- function(what, pkgs, linter_name, ...) {
@@ -154,7 +186,9 @@ run_on <- function(what, pkgs, linter_name, ...) {
154186

155187
linter <- get(linter_name)()
156188

157-
run_lints(pkgs, linter)
189+
check_depends <- linter_name %in% c("object_usage_linter", "object_name_linter")
190+
191+
run_lints(pkgs, linter, check_depends = check_depends)
158192
}
159193

160194
run_pr_workflow <- function(linter_name, pkgs, pr) {
@@ -193,4 +227,4 @@ if (is_branch) {
193227
lints <- purrr::map_df(linter_names, run_pr_workflow, packages, pr)
194228
}
195229

196-
write.csv(lints, outfile, row.names = FALSE)
230+
write.csv(lints, params$outfile, row.names = FALSE)

0 commit comments

Comments
 (0)