Switch branch detection to respect R_BIOC_VERSION #580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@jimhester once again, thanks for your replies on #578. I wanted to follow up here - if the intention of the environment variable R_BIOC_VERSION is to force a specific bioconductor version, I believe it should fully respect that setting in bioconductor_branch() too, which this PR adds. Unless I'm missing something, currently even if R_BIOC_VERSION is set, bioconductor_branch() is always going to pull from the git release branch. This is a very discrete update that defers to R_BIOC_VERSION if set. |
jimhester
reviewed
Feb 8, 2021
R/install-bioc.R
Outdated
| @@ -254,10 +254,12 @@ bioconductor_branch <- function(release, sha) { | |||
| sha | |||
| } else { | |||
| if (is.null(release)) { | |||
| release <- "release" | |||
| release <- ifelse(Sys.getenv("R_BIOC_VERSION") != "", yes = Sys.getenv("R_BIOC_VERSION"), no = "release") | |||
There was a problem hiding this comment.
Suggested change
| release <- ifelse(Sys.getenv("R_BIOC_VERSION") != "", yes = Sys.getenv("R_BIOC_VERSION"), no = "release") | |
| release <- Sys.getenv("R_BIOC_VERSION", "release") |
There was a problem hiding this comment.
good point - changed
Respond to code review
jimhester
reviewed
Feb 8, 2021
|
Thanks, can you please add a bullet to NEWS? It should briefly describe the change and end with |
|
@jimhester OK, added NEWS |
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
@jimhester Thanks for replying on the issue. My original idea here involved making a redundant option() to hold the git branch used when querying bioconductor. This updated PR simply uses the already supported R_BIOC_VERSION environment variable, and ensures that when querying git we use a branch that respects that setting.
There seem to be tests that need updating, which i would do if this is something you'd consider.
The remotes/bioconductor configuration is complex and maybe I'm missing something. However, if R_BIOC_VERSION is supposed to force remotes to use a specific version it seems like it should be respected on this codepath too.