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.

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

remove check for unavailabe dependencies on ancient R #6362

Merged
merged 8 commits into from
Aug 21, 2024
Merged

Conversation

ben-schwen
Copy link
Member

Current CI ancient job is failing because new version of xts, and evaluate depend on R 4.0.0. This PR patches the CI job by installing fixed versions for certain packages, afterwards.

@MichaelChirico
Copy link
Member

This feels a bit fragile to me. I wonder what the practical use case we're testing for is exactly:

  1. User on ancient R is also working with ancient {knitr}
  2. User on ancient R doesn't need to check vignettes, doesn't care about {knitr}

I've worked towards the latter in the R-CMD-check-occasional GHA, and taken the approach to just skip stuff related to these Suggests deps that won't install:

if (!do_vignettes) {
message("Skipping vignettes since knitr is unavailable.")
build_args = "--no-build-vignettes"
check_args = c(check_args, "--no-build-vignettes", "--ignore-vignettes")
}

(I don't really have a sense one way or the other what's more likely)

@ben-schwen
Copy link
Member Author

  1. User on ancient R is also working with ancient {knitr}
  2. User on ancient R doesn't need to check vignettes, doesn't care about {knitr}

Number 2 seems the way to go and also simplifies it!

.gitlab-ci.yml Outdated Show resolved Hide resolved
ben-schwen and others added 4 commits August 20, 2024 08:15
Copy link
Member

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

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

LGTM! May as well merge #6383 first, right? Since we know CI will fail on 3.3.0 at current master.

@ben-schwen
Copy link
Member Author

LGTM! May as well merge #6383 first, right? Since we know CI will fail on 3.3.0 at current master.

Yes, CI will only deliver new insights after merging #6383

@ben-schwen ben-schwen changed the title add fixed version for old r version remove check for unavailabe dependencies on ancient R Aug 20, 2024
@ben-schwen ben-schwen merged commit d66e12b into master Aug 21, 2024
7 checks passed
@ben-schwen ben-schwen deleted the ancient_ci branch August 21, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants