-
Notifications
You must be signed in to change notification settings - Fork 341
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
Problem: x/supply
is not marked as deprecated
#724
Problem: x/supply
is not marked as deprecated
#724
Conversation
Solution: Marked `x/supply` grpc and cli as deprecated. Fixes crypto-org-chain#720.
Codecov Report
@@ Coverage Diff @@
## master #724 +/- ##
==========================================
+ Coverage 19.05% 22.24% +3.19%
==========================================
Files 69 69
Lines 8000 8131 +131
==========================================
+ Hits 1524 1809 +285
+ Misses 5969 5772 -197
- Partials 507 550 +43
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -20,7 +20,6 @@ import ( | |||
"google.golang.org/grpc" | |||
"google.golang.org/grpc/codes" | |||
"google.golang.org/grpc/grpclog" | |||
"google.golang.org/grpc/metadata" |
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.
what are these changes in the generated pb?
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.
Not sure. Should I revert these? Integration tests seems to work fine with these changes.
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.
Maybe because of different protoc
version on my machine?
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'm not sure whether these metadata settings are anyhow critical for e.g. security headers or if one puts it behind LB. @yihuang do you know?
maybe the best is to align it with how the generated code is in other projects? (cronos, ethermint, Cosmos SDK...)
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've confirmed in Cosmos SDK. .pb.gw.go
files in Cosmos SDK also doesn't have import for google.golang.org/grpc/metadata
and any function call to NewContextWithServerTransportStream
. (Verified with code on tag v0.45.1
)
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.
Merging this PR.
Solution: Marked
x/supply
grpc and cli as deprecated. Fixes #720.