-
-
Notifications
You must be signed in to change notification settings - Fork 133
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
Delegate installation/configuration to rstantools #574
Conversation
There are a lot of whitespace changes due to my IDE stripping trailing whitespace, so it might be easier to review with the whitespace changes filtered out: https://github.com/stan-dev/rstanarm/pull/574/files?diff=split&w=1 Once this merged into the master branch, I'll work with @sambrilleman on integrating with the |
Thanks @andrjohns! We've been wanting to move to using rstantools for this for a while! I don't see anything that immediately jumps out to me as problematic, but this is more in @bgoodri's wheelhouse than mine, so we should have him review this too. @bgoodri Can you take a look at this when you have a chance? It would be great to merge this soon so that so that @andrjohns can work with @sambrilleman on getting the survival branch ready. |
Also @andrjohns I just added you to the r-packages team in the stan-dev organization so you can more easily contribute to whichever R package you want (thanks for all the help with the R packages!). You should be able to make branches and do maintenance stuff, etc. (not that you have to!). |
I'll look at it. The rstanarm package is hard because it has to be compiled
with LTO on Windows.
|
@bgoodri any chance you've had time to have a look at this? It would be great to get 2.26+ compatibility in-place for |
I haven't yet, but I will after I deal with some final exam stuff
…On Mon, Dec 12, 2022 at 11:29 AM Andrew Johnson ***@***.***> wrote:
@bgoodri <https://github.com/bgoodri> any chance you've had time to have
a look at this? It would be great to get 2.26+ compatibility in-place for
rstanarm
—
Reply to this email directly, view it on GitHub
<#574 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZ2XKTAB56AMOZKARIYFVLWM5HGJANCNFSM6AAAAAAQMHYL64>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@bgoodri Would you still have time to look at this? We'll need it for 2.26+ |
I have been looking at the ones for other packages, and since this one is similar, I am sure it will be fine. I am a little bit concerned about pending pull requests that branched off rstanarm a long time ago. I guess it could be managed but when those get merged, they might put stuff in the old locations or restore deleted files. |
@bgoodri if you're happy with this PR would you mind merging and putting together a new CRAN release? I'm happy to update/check the current pending PR's for any conflicts |
Closing in favour of #587, will reopen once that PR is merged and I've resolved the merge conflicts with this one |
This PR updates the
rstanarm
installation to fully delegate torstantools
. This will help ensure ongoing compatibility withrstan
and its updates, and simplify some of the maintenance involved with the package.The PR also updates the Github actions workflow to test against more combinations of R and OS versions