-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Move /bank/balances/{address} REST endpoint into /x/bank #4570
Conversation
x/bank now provides a Querier too. Closes: #4560
6048e9f
to
e45f12a
Compare
Codecov Report
@@ Coverage Diff @@
## master #4570 +/- ##
==========================================
+ Coverage 52.65% 52.68% +0.03%
==========================================
Files 261 263 +2
Lines 16410 16427 +17
==========================================
+ Hits 8640 8655 +15
- Misses 7124 7125 +1
- Partials 646 647 +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.
LGTM! Just a minor nitpick 👍
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.
why don't we remove this endpoint all together? I really don't see the need of keeping it as we already have that functionality
Co-Authored-By: Alexander Bezobchuk <[email protected]>
Because clients depend on this endpoint @fedekunze :) |
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.
ACK
@@ -17,6 +16,7 @@ import ( | |||
// RegisterRoutes - Central function to define routes that get registered by the main application | |||
func RegisterRoutes(cliCtx context.CLIContext, r *mux.Router) { | |||
r.HandleFunc("/bank/accounts/{address}/transfers", SendRequestHandlerFn(cliCtx)).Methods("POST") | |||
r.HandleFunc("/bank/balances/{address}", QueryBalancesRequestHandlerFn(cliCtx)).Methods("GET") |
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.
that's a much better place
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.
ACK
Closes: #4560
docs/
)clog add [section] [stanza] [message]
Files changed
in the github PR explorerFor Admin Use: