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

refactor: Using submodules as features #460

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

luiz10x
Copy link
Contributor

@luiz10x luiz10x commented Nov 12, 2021

Following the discussion in #342, this PR tries to get an initial MVP for (potentially) splitting into subcrates in the future.

The important change is in Cargo.toml: each submodule becomes a feature, and they are all enabled by default. In turn, all the dependencies are optional, and I mapped what deps are used for each submodule in the correspondent feature.

The main advantage of this approach is not making maintenance harder (it is still all one crate, with one version). It is a bit weird to have all the deps as optional, but I don't think that is too bad?

I also keep all new submodule features in the default feature set, so current users won't see a difference. In the future it might be worth adding docs/instructions to guide people in two directions:

  • if you're using bio in an application, using the default bio = "0.x" dependency is fine; you might want to tune to reduce your build times, but it won't make it harder for new users to use rust-bio.
  • for library writers disabling default features and using only what you need is recommended, because your choice of features has consequences for people using your library. In this case you might prefer to use bio = { version = "0.x", default-features = false, features = ["io"] }" if you only use the io features.

I also added extra CI checks for running tests for each feature/submodule individually, to guarantee they stay working (and minimally depend on each other).

Points for discussion

  • the phylogeny feature was not tested in CI, and a docstring test was actually broken. I added a CI check, and since it is inside the io submodule I also made it activate the io feature if used.

Future work

  • this opens the way to make serde into a feature too, but I didn't want to change the code too much
  • newtype_derive and custom_derive are unmaintained (last releases are from many years ago), maybe find alternatives?

@luiz10x luiz10x changed the title Using submodules as features refactor: Using submodules as features Nov 12, 2021
@coveralls
Copy link

coveralls commented Nov 12, 2021

Coverage Status

Coverage remained the same at 88.675% when pulling 5491aab on luiz10x:lirber/features into 9e8a7ca on rust-bio:master.

@allan2
Copy link

allan2 commented Nov 24, 2021

I think splitting of the crates as discussed in #342 is a good idea.

However, I don't think it is helpful to turn submodules into a feature as a "middle step".
We should figure out how to split the crates properly, since the submodules as they are now may not be how we want the smaller crates to be organized.

Continuing discussion at #342

@johanneskoester
Copy link
Contributor

johanneskoester commented Dec 6, 2021

Interesting idea @luiz10x! Please see my comment in the discussion issue.

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

4 participants