Conversation
LucioFranco
left a comment
There was a problem hiding this comment.
LGTM just want to see the eq stuff backed out into codegen and have one qusetion
tonic-build/src/prost.rs
Outdated
| let type_attributes = vec![( | ||
| ".".to_string(), | ||
| "#[allow(clippy::derive_partial_eq_without_eq)]".to_string(), | ||
| )]; |
There was a problem hiding this comment.
As adding attributes might only be configured via prost api, it seems like we might need to configure in this way. However I'm not sure that this is an appropriate approach.
There was a problem hiding this comment.
Ugh I wonder if we should just fix this in prost then? Somewhere in this macro impl https://github.com/tokio-rs/prost/blob/master/prost-derive/src/lib.rs#L21 since really the problem is I don't want code generated stuff to impl Eq or we can put this in prost-build?
There was a problem hiding this comment.
I published tokio-rs/prost#775 so as to try moving this to prost-build.
|
I'll remove clippy attributes from each fille which includes proto (they were added in this pull request), and update generated codes. |
|
The ci failure is interesting I think maybe you didn't run it with the clean up feature enabled? |
|
I marked this pull request as draft because of using unreleased prost-build (tokio-rs/prost#775). I'll remove this and update prost-build when it is released. I fixed the codegen in 23ebd27, and updated generated codes in 68b61d0. Thanks for your advice. |
|
Working on that now tokio-rs/prost#777 |
|
ok its been published as v0.11.4 |
…xtern_path test" This reverts commit 7fe1d32.
tests/root-crate-path/src/main.rs
Outdated
| @@ -1,4 +1,4 @@ | |||
| #[derive(Clone, PartialEq, ::prost::Message)] | |||
| #[derive(Clone, PartialEq, Eq, ::prost::Message)] | |||
There was a problem hiding this comment.
This should get an allow with the new version of prost right?
There was a problem hiding this comment.
This looks like my mistake. I'll fix this.
Signed-off-by: Gaius <gaius.qi@gmail.com>
Fixes clippy lints.