-
Notifications
You must be signed in to change notification settings - Fork 93
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
chore(r/adbcsnowflake): Update adbcsnowflake such that it can be submitted to CRAN #1220
Conversation
Actually, I think I know why the files have to be modified: I was using the C++ version of helpers.h, and |
Most of these problems will go away when the vendored code is downloaded at runtime if needed, but I ran it through winbuilder and:
|
(CI failures are because https://r-project.org is down which has halted R CI jobs pretty much everywhere.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments. lgtm, I did not look at the test changes.
@@ -17,9 +17,9 @@ Encoding: UTF-8 | |||
Roxygen: list(markdown = TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not missing a lower bound when you mention below that this will not compile on <4.2?
Roxygen: list(markdown = TRUE) | |
Depends: R (>= 4.2) | |
Roxygen: list(markdown = TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to find that comment to clarify...the 4.2 version constraint is just for Windows.
Closes #1067.
The mailing list thread where I asked about this ( https://stat.ethz.ch/pipermail/r-package-devel/2023q3/009329.html ) noted a few possible requirements:
file://
URL...when submission times come that URL and shasum can be hardwired into the package.I also added some updates to make the tests run with my local Snowflake account and consoliated the Windows and MacOS and Linux builds since the all use the same logic.
This will probably need to be workshopped during submission and, after successful submission, distilled into some common place and applied to the adbcflightsql package.