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

Rework features param in rust_source() #249

Merged

Conversation

Ilia-Kosenkov
Copy link
Member

@Ilia-Kosenkov Ilia-Kosenkov commented Mar 5, 2023

A general improvement of rust_source().

  • Require extendr_deps and dependencies to be named lists
  • Merge (instead of appending) extendr_deps into dependencies -- if dependencies contains extendr-api entry, it will be overwritten by that of extendr_deps
  • Make features a feature flag variable, allowing "ndarray", "num-complex", "serde", "graphics" features (and their combinations)
  • Correctly set features property on extendr-api no matter how it is defined
  • Add explicit references to relevant packages for all features but graphics

Fixed #239

@Ilia-Kosenkov Ilia-Kosenkov marked this pull request as ready for review March 5, 2023 22:35
@Ilia-Kosenkov Ilia-Kosenkov changed the title Reinvent features param in rust_source() Rework features param in rust_source() Mar 5, 2023
R/generate_toml.R Outdated Show resolved Hide resolved
R/generate_toml.R Outdated Show resolved Hide resolved
R/generate_toml.R Outdated Show resolved Hide resolved
R/generate_toml.R Outdated Show resolved Hide resolved
R/generate_toml.R Outdated Show resolved Hide resolved
tests/data/either_example.rs Outdated Show resolved Hide resolved
tests/testthat/test-optional-features.R Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
Co-authored-by: Michael Milton <ttmigueltt@gmail.com>
@CGMossa
Copy link
Member

CGMossa commented Mar 6, 2023

Please rename the example that doesn't contain either, to something without either.

NEWS.md Outdated Show resolved Hide resolved
Co-authored-by: CGMossa <cgmossa@gmail.com>
Copy link
Member

@multimeric multimeric left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@multimeric multimeric left a comment

Choose a reason for hiding this comment

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

Basically all my suggestions are style things. I have no actual criticisms.

R/features.R Outdated Show resolved Hide resolved
R/features.R Outdated Show resolved Hide resolved
R/features.R Outdated Show resolved Hide resolved
R/features.R Outdated Show resolved Hide resolved
R/features.R Outdated Show resolved Hide resolved
Ilia-Kosenkov and others added 2 commits March 7, 2023 19:16
Co-authored-by: Michael Milton <ttmigueltt@gmail.com>
@Ilia-Kosenkov Ilia-Kosenkov merged commit 110e53d into extendr:main Mar 7, 2023
@Ilia-Kosenkov Ilia-Kosenkov deleted the 239-correct-extendr-features branch September 24, 2023 16:53
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.

rust_source()'s features argument seems to be misleading
3 participants