Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let use_package() decrement a package dependency. #2043

Merged
merged 7 commits into from
Aug 15, 2024
Merged

Let use_package() decrement a package dependency. #2043

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6d8e65d
Let `use_package()` decrement a package dependency. Fixes #1957.
jplecavalier Aug 15, 2024
692c8cd
Merge branch 'main' into decrement-min-version
jplecavalier Aug 15, 2024
ceef905
Using if () ... else () instead of ifelse() on non-vectorized operation
jplecavalier Aug 15, 2024
db7d98e
Merge branch 'main' into decrement-min-version
jplecavalier Aug 15, 2024
548e5ab
Style
jennybc Aug 15, 2024
4278d10
Simplify glue string
jennybc Aug 15, 2024
91a7cad
Credit @jplecavalier for the PR
jennybc Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# usethis (development version)

## Bug fixes and minor improvements

* `use_package()` now decreases a package minimum version required when
`min_version` is lower than what is currently specified in the DESCRIPTION
file (@JosiahParry, #1957).

* Reverse dependency checks are only suggested if they exist
(#1817, @seankross).

Expand Down
25 changes: 13 additions & 12 deletions R/helpers.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,19 @@ use_dependency <- function(package, type, min_version = NULL) {
"!" = "Package {.pkg {package}} is already listed in
{.field {existing_type}} in DESCRIPTION; no change made."
))
} else if (delta == 0 && !is.null(min_version)) {
} else if (delta == 0 && version_spec(version) != version_spec(existing_version)) {
# change version
upgrade <- existing_version == "*" ||
numeric_version(min_version) > version_spec(existing_version)
if (upgrade) {
ui_bullets(c(
"v" = "Increasing {.pkg {package}} version to {.val {version}} in
DESCRIPTION."
))
desc$set_dep(package, type, version = version)
desc$write()
changed <- TRUE
}
direction <- ifelse(version_spec(version) > version_spec(existing_version),
"Increasing", "Decreasing")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this to an if () ... else? I generally reserve ifelse() for vectorized situations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!


ui_bullets(c(
"v" = "{.drn {direction}} {.pkg {package}} version to {.val {version}} in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the .drn inline markup. Can you help me understand that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried to replicate the same logic as in {.pkg {package}} and{.val {version}}. I tried to remove .drn and then {direction} would not resolve with the string in the object direction. Any hint?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like {.pkg {package}} does interpolation of package through its surrounding brackets. Then .pkg and the second layer of brackets applies some inline styling.

For pure interpolation (which is the goal with direction), we just need one pair of brackets.

You can learn more about this in the cli package: https://cli.r-lib.org/reference/inline-markup.html.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out, and thanks again for your time and patience today!

DESCRIPTION."
))
desc$set_dep(package, type, version = version)
desc$write()
changed <- TRUE

} else if (delta > 0) {
# moving from, e.g., Suggests to Imports
ui_bullets(c(
Expand All @@ -96,6 +96,7 @@ r_version <- function() {
}

version_spec <- function(x) {
if (x == "*") x <- "0"
x <- gsub("(<=|<|>=|>|==)\\s*", "", x)
numeric_version(x)
}
Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/_snaps/helpers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@
Message
v Increasing crayon version to ">= 2.0.0" in DESCRIPTION.

---

Code
use_dependency("crayon", "Imports", min_version = "1.0.0")
Message
v Decreasing crayon version to ">= 1.0.0" in DESCRIPTION.

# use_dependency() upgrades a dependency

Code
Expand Down
4 changes: 3 additions & 1 deletion tests/testthat/test-helpers.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ test_that("we message for version change and are silent for same version", {
expect_snapshot(
use_dependency("crayon", "Imports", min_version = "2.0.0")
)
expect_silent(use_dependency("crayon", "Imports", min_version = "1.0.0"))
expect_snapshot(
use_dependency("crayon", "Imports", min_version = "1.0.0")
)
})

## https://github.com/r-lib/usethis/issues/99
Expand Down
Loading