-
Notifications
You must be signed in to change notification settings - Fork 228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proto: re-export prost_types
as well_known_types
#658
Conversation
My main interest is being able to get to `prost_types::Timestamp` without having to include `prost_types` in addition to `tendermint-proto`, however I think there might be more cases where it's helpful to have access to the well-known proto types in the future.
1afaa33
to
c1cc891
Compare
Codecov Report
@@ Coverage Diff @@
## master #658 +/- ##
========================================
+ Coverage 43.4% 43.7% +0.3%
========================================
Files 169 175 +6
Lines 11828 11898 +70
Branches 2721 2720 -1
========================================
+ Hits 5141 5208 +67
- Misses 6410 6411 +1
- Partials 277 279 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that makes sense! LGTM 👍 I'll let Greg have the final say on this PR as I'm bit out of loop on everything proto-related currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it helps you @tony-iqlusion, it makes sense to me 😁
@greg-szabo, any objections from your side?
Looks like this is also addressed in #653? (at least according to @greg-szabo although I'm not quite sure where/how) |
Apologies, I gave Tony the wrong PR number. In #639 we dissected prost_types and added our own implementation so they can be serialized as JSON objects. So, there is a But we are trying to separate protobuf and JSON serialization in an upcoming RC and it's possible that we won't need to dissect To keep things manageable (although not the simplest), I suggest that you use the But I would rather not add both to RC2 to avoid confusion. If anyone has strong opinions in adding it to RC2, please raise it here. Maybe I'm the only one who would get confused. :) |
|
My main interest is being able to get to
prost_types::Timestamp
without having to includeprost_types
in addition totendermint-proto
, however I think there might be more cases where it's helpful to have access to the well-known proto types in the future.