search

fdeantoni / prost-wkt

Prost Well-Known-Types serialization and deserialization.
Apache License 2.0
75 stars 37 forks source link

`unwarp()` in generated code #3

Closed gorzell closed 3 years ago

gorzell commented 3 years ago

Thanks for taking the time to add support for this functionality. I was wondering if you would be willing to either use expect instead of unwrap in the generated code, or explicitly add the #[allow(clippy::unwrap_used)] macro. We happen to have the "no unwrap" calls lint turned on (our thinking is that outside of tests, expect is always better than unwrap) so the generated code was breaking our build.

There is another work around where you can disable the lint for the entire module:

// The generated code has some calls to unwrap, so we need to suppress that lint here.
#[allow(clippy::unwrap_used)]
        pub mod v1 {
            include!(concat!(env!("OUT_DIR"), "/foo.v1.rs"));
        }

But that feels a bit heavy handed.

fdeantoni commented 3 years ago

Hi @gorzell , many thanks for your feedback! It is definitely better to use expect instead. I should actually have no unwrap or expect at all in the code :) I will go ahead and change those unwraps to expectinstead, and at the same time will probably add a new method which will return a Result instead (I think Prost DecodeError has been made public again).

gorzell commented 3 years ago

Sounds good, thanks for the quick response.

fdeantoni commented 3 years ago

I've created a new release (0.3.0) that hopefully addresses this problem. It also deprecates pack and unpack in favour of try_pack and try_unpack.

gorzell commented 3 years ago

Nice! I updated our code base, and removed the lint exception. We don't have exhaustive tests, but those that we do have seem to be working. Thanks again.