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

Add devel-ucrt to CI matrix #296

Merged
merged 1 commit into from Sep 13, 2021
Merged

Add devel-ucrt to CI matrix #296

merged 1 commit into from Sep 13, 2021

Conversation

jeroen
Copy link
Contributor

@jeroen jeroen commented Sep 10, 2021

This may be useful to ensure that your rwinlib tiledb binaries are working as expected also for ucrt.

The R-devel-ucrt installer from Tomas currently has special built-in repos, so I had to remove the line where you override this. And also I had to remove Matrix because this is a recommended package, which is apparently not in the ucrt repo.

@eddelbuettel
Copy link
Member

Thank you -- saw the fork (and its inital fail on package ps but yes, this should be useful.

.github/workflows/windows.yaml Show resolved Hide resolved
install.packages(c("remotes", "rcmdcheck"))
remotes::install_deps(dependencies = NA)
install.packages(c("Matrix", "palmerpenguins", "nycflights13", "tibble", "data.table"))
install.packages(c("palmerpenguins", "nycflights13", "tibble", "data.table"))
Copy link
Member

Choose a reason for hiding this comment

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

My bad. I guess Matrix is always a given even if only 'Recommended'. I am tainted from (Debian's) r-base-core working fine without it so I have a habit of adding it.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

LGTM. One small question, but feel free to merge and we can change in follow-up if needed.

.github/workflows/windows.yaml Show resolved Hide resolved
@jeroen
Copy link
Contributor Author

jeroen commented Sep 13, 2021

@jeroen any reason not to keep one on windows-latest?

This allows us to actually test the package in native UTF-8 locale, which has been the main motivation for R to switch to ucrt: https://docs.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page#set-a-process-code-page-to-utf-8

It is also the version CRAN will be testing on: https://cran.r-project.org/web/checks/check_flavors.html

@ihnorton
Copy link
Member

@jeroen sorry what I meant was windows-2019 -> windows-latest because IIUC they are currently the same thing? Anyway, let's merge as-is; GH is aggressive with their image pruning but I assume windows 2019 will be around for at least a few more years.

@eddelbuettel eddelbuettel merged commit 4c184f6 into TileDB-Inc:master Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants