Implement TryFrom<Builder> for Target for all builders exposing a public build method

[TedDriggs]

With #72 (fallible setters) merged and rust-lang/rust#33417 (TryFrom) approaching stabilization, having a TryFrom<Builder> for Target implementation for all builders with a generated build method will enable the following code:

let request = RequestBuilder::default()
    .from(-18000000)
    .sources(vec![Source::device_group(Oid::new(2))])
    .metric(Metric::builder()
        .stat_name("extrahop.device.ssl_server_detail")
        .field_name("cert_expiration"))?
    .total(true)
    .build()
    .expect("All inputs hardcoded");

Once TryFrom is stabilized I'm not planning to expose a struct-level attribute to control this. This seems too standard for a crate user not to want. #70 gives authors control over exposing the build method; this would look to that attribute and skip implementation if the build method was skipped.

I'm planning to take a stab at a PR for this.

[colin-kiegel]

Makes sense 👍

I generally agree that we should always emit that implementation. There is only one reason to make it opt-into and that is backwards compatibility with Rust 1.15-x. I don't want to drop backwards compatibility on every new cool feature. For some intermediate time I would like to make it opt-in (either feature flag or attribute). And after some time we can raise the minimum Rust version and get rid of opting-in. I guess a feature flag would be a little easier to implement? With this we can already implement it for nightly. :-)

Minor corrections in your code example

• You meant RequestEnvelope::try_from
• the build().expext("..") calls should not be there
• Metric::builder() would currently still be MetricBuilder::default()

[TedDriggs]

• The expect call should unwrap the build result with a nicer panic message. In this case, there was no RequestEnvelopeBuilder, but there was a hand-rolled From impl.
• See point 1
• Oops; I've added that by hand in the crate where this was copy-pasted from. Optionally expose a static Target::builder() -> TargetBuilder method #73

I went with feature flag.

[colin-kiegel]

Ah .. I see .. I thought the outer RequestEnvelope::from(RequestBuilder should've also been a demonstration of the usefulness of the automatic TryFrom implementation.

Ok, that was fast. I'll review later. Have to got now. :-)

[TedDriggs]

TryFrom is now supported below this crate's MSRV, so this should be ready to implement.

[MrBeeMovie]

Is this still being looked at? Would love if the From and TryFrom traits were implemented.

[TedDriggs]

It is not actively being looked at.

• We wouldn't support From given that building is typically fallible.
• Supporting TryFrom is possible, but may be fiddly due to all the settings that now flow through the build_fn. Given that it's very short to add this directly to source code, I don't think it's a priority, but would be open to reviewing a PR for it, as long as that PR had good test coverage.

[TedDriggs]

The lack of interest in this over the years makes me think it's not worth the complexity of implementation.