Feature use-serde renamed to serde
#1006
Conversation
Cargo.toml
Outdated
| @@ -18,7 +18,7 @@ default = [ "std", "secp-recovery" ] | |||
| base64 = [ "base64-compat" ] | |||
| unstable = [] | |||
| rand = ["secp256k1/rand-std"] | |||
| use-serde = ["serde", "bitcoin_hashes/serde", "secp256k1/serde"] | |||
| serde = ["actual_serde", "bitcoin_hashes/serde", "secp256k1/serde"] | |||
There was a problem hiding this comment.
nit: Perhaps use - instead of _ to be consistent with other names?
There was a problem hiding this comment.
+1 from me (I often wish we had bitcoin-hashes too but its probably too much churn to replace bitcoin_hashes).
There was a problem hiding this comment.
My naming is usually _ where I would've put space and - where I would've put - in normal text. Although it sometimes isn't a clear cut - e.g. bitcoin_hashes :D
Anyway, will rename, I also forgot to document people must not rely on the name.
There was a problem hiding this comment.
I like - because it reminds me of writing Lisp :)
Looks like you changed it in the manifest but did not change all the actual_serde instances in the derives. Was that intentional?
There was a problem hiding this comment.
Yes, it has to be _ in the code since extern crate actual-serde; wouldn't compile.
Cargo.toml
Outdated
| @@ -18,7 +18,7 @@ default = [ "std", "secp-recovery" ] | |||
| base64 = [ "base64-compat" ] | |||
| unstable = [] | |||
| rand = ["secp256k1/rand-std"] | |||
| use-serde = ["serde", "bitcoin_hashes/serde", "secp256k1/serde"] | |||
| serde = ["actual_serde", "bitcoin_hashes/serde", "secp256k1/serde"] | |||
There was a problem hiding this comment.
+1 from me (I often wish we had bitcoin-hashes too but its probably too much churn to replace bitcoin_hashes).
| @@ -41,6 +41,7 @@ use crate::VarInt; | |||
| /// the actual transactions | |||
| #[derive(Copy, PartialEq, Eq, Clone, Debug)] | |||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | |||
| #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))] | |||
There was a problem hiding this comment.
Ahh, that is what was needed. Thanks for figuring this bit out.
Features activating external crates are supposed to have same name as those crates. However we depend on same feature in other crates so we need a separate feature. After MSRV bump it is possible to rename the crates and features so we can now fix this inconsistency. Sadly, derive can't see that the crate was renamed so all derives must be told to use the other one.
|
I agree with comments preferring |
…`serde` 2e7effc Feature `use-serde` renamed to `serde` (Martin Habovstiak) Pull request description: Features activating external crates are supposed to have same name as those crates. However we depend on same feature in other crates so we need a separate feature. After MSRV bump it is possible to rename the crates and features so we can now fix this inconsistency. Sadly, derive can't see that the crate was renamed so all derives must be told to use the other one. Replaces #373 ACKs for top commit: apoelstra: ACK 2e7effc Tree-SHA512: b20364b9e8f30c2269bef915e821b2b2ec929e71dd0e88af2bc3a021821f87011d35e095cb8efe99add77a23dde940a17537eb387fb4582b05c57c8679969eb0
Features activating external crates are supposed to have same name as
those crates. However we depend on same feature in other crates so we
need a separate feature. After MSRV bump it is possible to rename the
crates and features so we can now fix this inconsistency.
Sadly, derive can't see that the crate was renamed so all derives must
be told to use the other one.
Replaces #373