parity-common
parity-common copied to clipboard
Add support for valuable to fixed hashes
Value::String
was used for the visit implementation, because it's the most convienient format to use while tracing, AFAIK being the main motivation for the valuable
crate.
ping
Hey @marmistrz. Could you clarify what you need this for exactly?
it's the most convienient format to use while tracing
what kind of tracing are you doing?
I'm hesitant to add yet another feature that will introduce unnecessary breaking changes to our crates unless it's really justified.
This is not a breaking change: the trait implementation will only be used if the feature is enabled.
Tracing is a framework for instrumenting Rust programs to collect structured, event-based diagnostic information.
See https://tokio.rs/blog/2021-05-valuable
Without this change it's impossible to use #[derive(Valuable)]
on any type (struct
/enum
) containing any parity primitive type.
This is not a breaking change: the trait implementation will only be used if the feature is enabled.
I meant that when valuable
version bumps to 0.2, updating fixed-hash to 0.2 would be a breaking change (unless they do it in a backwards compatible way for the current impl and we specify >=0.1 < 0.3
in Cargo.toml
). So we're tying yet another dependency's breaking change to our crate.
There are requests to add other trait impls (e.g. #640) and at the moment, I'm trying to conservative and not add something unless it is really needed.
We could use a type like serde_json::Value to pass arbitrary structured data but this would require allocating and copying the data from our application's struct to hand it to the collector.
We already provide serde
implementation if this Valuable
is needed to avoid an allocation and copying the data in this case, I'm yet to be convinced we need it.
If there's consensus we do want to add support for valuable
, it should be added to other crates like uint
as well.
I see. This implementation has to either be a part of valuable
or primitive-types
due to orphan rules. So far tracing doesn't support Value::from_serde
.