Skip to content
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

Implement client functionality for the community pool #3939

Merged
merged 9 commits into from
Mar 28, 2019

Conversation

jackzampolin
Copy link
Member

Fixes: #3937

@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #3939 into develop will increase coverage by <.01%.
The diff coverage is 71.42%.

@@             Coverage Diff             @@
##           develop    #3939      +/-   ##
===========================================
+ Coverage    60.33%   60.34%   +<.01%     
===========================================
  Files          196      196              
  Lines        14488    14495       +7     
===========================================
+ Hits          8742     8747       +5     
- Misses        5165     5166       +1     
- Partials       581      582       +1

@sabau
Copy link
Contributor

sabau commented Mar 20, 2019

I was proposing something similar, may I push here the tests?

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing pending log entry, otherwise LGTM 👍

x/distribution/client/cli/query.go Outdated Show resolved Hide resolved
@alexanderbez alexanderbez requested review from alessio and mossid March 20, 2019 15:38
Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addition affects the REST API as well. @jackzampolin can you update the REST docs and add an additional changelog entry under both features/gaiarest and features/gaiacli please?

@rigelrozanski
Copy link
Contributor

@jackzampolin why'd delete the PR checkboxes ;)

x/distribution/keeper/querier_test.go Outdated Show resolved Hide resolved
x/distribution/keeper/querier_test.go Outdated Show resolved Hide resolved
x/distribution/keeper/querier.go Show resolved Hide resolved
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)

res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/community_pool", queryRoute), nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's weird that in CLI queries we sometimes use routes and sometimes the key (with cliCtx.QueryStore()

rigelrozanski and others added 3 commits March 26, 2019 09:43
Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>
Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>
- Add new route to swagger.yaml
- Add community pool command to docs
- added pending changes

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
@alessio alessio merged commit 38b7c07 into develop Mar 28, 2019
@alessio alessio deleted the jack/community-pool branch March 28, 2019 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:CLI C:x/distribution distribution module related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants