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

Reorganise GitHub Actions #204

Open
CGMossa opened this issue Nov 12, 2023 · 6 comments
Open

Reorganise GitHub Actions #204

CGMossa opened this issue Nov 12, 2023 · 6 comments

Comments

@CGMossa
Copy link
Member

CGMossa commented Nov 12, 2023

I'd like to define workflows here that I see are present, so we can clean up the current CI we have going on.

For routine maintenance

  1. Generate bindings from new versions of R, R-devel, etc.
  2. nonAPI.txt also changes with these versions, and there is an action for it

For PR authors (contributors outside & inside the org)

  1. Run tests, layout tests, on the generated bindings due to their changes, that they have not seen before, because they don't have all platforms available to them, nor all R versions installed.
  2. Append their PRs with generated bindings, similar to (1), but should refer to current PR and push to current PR branch.

For maintainers / ambitious PR authors:

  1. Run extendr main on PR'd generated bindings, I believe this is what is meant with Run bindings tests on freshly generated bindings #162
@Ilia-Kosenkov
Copy link
Member

Why separate 5 from a regular routine?

@CGMossa
Copy link
Member Author

CGMossa commented Nov 13, 2023

I mean it is up for discussion.

I don't think every contribution warrants acceptance, based on its compatibility with current version of extendr. I don't think it is a burden to be put on outside contributors, for every little thing they PR. It might be up to maintainers / authors to benchmark this, or usher it in with upstream (to extendr) PRs, but not all contributions or contributors should be slammed with this.

There are thousands of improvements that could be done here, but we have interlocked the process between these two repos, I'd say, beyond reason.

@yutannihilation
Copy link
Contributor

IMHO, libR-sys is not the type of crate that has frequent updates except for regenerating the bindings against the latest version of R. So, I'm afraid optimizing it for pull requests is not worth. Frankly, I believe libR-sys should not be updated unless there's some strong reason.

For point 5, I guess the issue refers to tests on libR-sys, not on extendr. See #157.

@CGMossa
Copy link
Member Author

CGMossa commented Nov 16, 2023

I disagree with this attitude in general. There are something wrong with the way github actions are setup at the moment. It blocks certain changes.

If libR-sys is only to serve extendr, then they should be merged. I believe if we had more users, there would be atleast monthly feature requests on this crate, and it is shockingly unfriendly to outsiders at the moment.

@yutannihilation
Copy link
Contributor

I believe if we had more users, there would be atleast monthly feature requests on this crate

My take is the opposite. What's important in a sys crate is not "features," but stability. I use libR-sys outside of extendr, but never felt I should file a feature request because the current libR-sys is sufficient to me.

But, yeah, in actual, I think you are right. There are few libR-sys users like me. I think I'll create a simpler alternative for myself, so that's fine.

@CGMossa
Copy link
Member Author

CGMossa commented Nov 17, 2023

Sure, I have similar opinions about stability.

But this crate is not a sys-crate for R bindings. This is a sys-crate for R-internals as of right now. A crate that wants to do different things then extendR would not find libR-sys to be living up to its name.

There have already been two instances, where people have asked about a certain capability, from libR-sys and been met with negative response. (1) We have someone writing an alternative R interpreter, and they wanted to piggyback on R packages, through libR-sys, but then libR-sys doesn't provide access to all the headers in R-API. (2) Another example was someone that wanted to use R's optimisation stuff in Rust, and they ended up attempting to redo bindings project, and I think they gave up at the end.

Dealing with these issues is not trivial. You and I spend a lot of energy on this. And I preface a lot of the discussions, that I'm not trying to "push" one direction or another.

One of the shortcomings of the current CI workflows is explained in #208. I tried to be as clear as possible in there.

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

No branches or pull requests

3 participants