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

Use automatic failure feature instead of redundant use_failure #15

Merged
merged 2 commits into from
Sep 29, 2019

Conversation

faern
Copy link
Contributor

@faern faern commented Sep 23, 2019

Optional dependencies automatically becomes a feature with the same name as that dependency. So it can be used instead of creating an arbitrary extra feature, and is IMO more idiomatic.

Strictly technically speaking this PR is a breaking change. But in practice I doubt it would break any existing code. It just renames a feature that is activated by default. So for people with the defaults they won't notice. And people deactivating default features hopefully never activated use_failure again, since that would simply just bring them back to where they started.

@harryfei
Copy link
Owner

Of course, the code will be more idiomatic after doing those changes.
However, it's a less obvious way to show the fact that which crate has a failure optional feature.
What do you think of this?

@faern faern force-pushed the use-automatic-failure-feature branch from e4e3fa1 to d08fd2f Compare September 27, 2019 06:47
@faern
Copy link
Contributor Author

faern commented Sep 27, 2019

I don't think it's less obvious. It's the idiomatic way most people do it. So Rust developers would be used to that pattern. The serde feature is a super common way to activate serialization support in a ton of crates. They don't have use_serde or serialization for example.

Since features are meant to be purely additive of features, it's basically redundant with the use_ part, since most features will activate the usage of something anyway. If I read features = ["failure"] I read it out like "Activate the failure feature" or "Use the failure feature". Which reads much more naturally than "Activate the use failure feature".

Cargo.toml in not really the place where people should discover how to use the libarary anyway. I think no matter the name of the feature in Cargo.toml things like these has to be written about in the readme/documentation. I added a section about it in this PR now.

@harryfei
Copy link
Owner

harryfei commented Sep 29, 2019

OK, I choose to accept this PR. Thanks for your work.

@harryfei harryfei merged commit 1419887 into harryfei:master Sep 29, 2019
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

2 participants