-
Notifications
You must be signed in to change notification settings - Fork 13
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
Enable default features in prost dependencies #1344
Conversation
Since we're not supporting `no_std` crates, we should enable the `std` feature by default.
Related public slack post. Ultimately, not all crates need the |
@@ -16,6 +16,7 @@ registry: | |||
- name: "prost" | |||
req: "0.12.3" | |||
default_features: false | |||
features: ["std"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this difference between this and setting default_features: true
that it would enable the derive
feature too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that's the difference. Not sure if we want to enable derive
or not. My vague understanding (from the README) is that it's more for user-generated types, but OTOH the custom derive
macro is added to the generated code.
Let's maybe just add it (by using default_features: true
) — it's a single extra crate with no dependencies.
@@ -16,6 +16,7 @@ registry: | |||
- name: "prost" | |||
req: "0.12.3" | |||
default_features: false | |||
features: ["std"] | |||
# prost-types is necessary for any module using the WKTs. | |||
- name: "prost-types" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do the same here? https://docs.rs/prost-types/0.13.1/prost_types/#feature-flags
std
feature to prost dependency
Since we're not supporting
no_std
crates, we should enable thestd
feature by default. (This happens to be the same asdefault_features
currently, but that may change in the future.)