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

min duration from days to hours, changed weekly to day #859

Merged
merged 2 commits into from
Feb 13, 2022

Conversation

czarcas7ic
Copy link
Member

Fixes two queries

osmosisd query lockup total-locked-of-denom gamm/pool/1
Wont work since it tries to use 1d as default min duration. Changed default to 86400s.

osmosisd query epochs current-epoch -h
Shows weekly as a potential input but its week not weekly. While I was at it I just changed default to day since it is most used.


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2022

Codecov Report

Merging #859 (01b53ef) into main (39c4457) will not change coverage.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #859   +/-   ##
=======================================
  Coverage   19.86%   19.86%           
=======================================
  Files         190      190           
  Lines       24763    24763           
=======================================
  Hits         4918     4918           
  Misses      18982    18982           
  Partials      863      863           
Impacted Files Coverage Δ
x/epochs/client/cli/query.go 0.00% <0.00%> (ø)
x/lockup/client/cli/flags.go 50.00% <50.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39c4457...01b53ef. Read the comment docs.

@@ -14,13 +14,13 @@ const (
func FlagSetLockTokens() *flag.FlagSet {
fs := flag.NewFlagSet("", flag.ContinueOnError)

fs.String(FlagDuration, "86400s", "The duration token to be locked. e.g. 1h, 1m, 1s, 0.1s")
fs.String(FlagDuration, "86400s", "The duration token to be locked. e.g. 86400s, 604800s, 1209600s")
Copy link
Member

Choose a reason for hiding this comment

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

Can we put these in hours? I believe thats the largest default unit supported by golang

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

TY, LGTM! Can we change the units to hours, since its easier to understand than seconds?

@czarcas7ic czarcas7ic changed the title min duration from 1d to 86400s, changed weekly to day min duration from days to hours, changed weekly to day Feb 13, 2022
@czarcas7ic czarcas7ic merged commit e4e97a3 into main Feb 13, 2022
@czarcas7ic czarcas7ic deleted the adam/query-fix branch February 13, 2022 23:31
@ValarDragon ValarDragon added the A:backport/v6.x backport patches to v6.x branch label Feb 13, 2022
mergify bot pushed a commit that referenced this pull request Feb 13, 2022
* min duration from 1d to 24h (d not recognized), changed weekly to day (weekly doesnt work, only week)

* changed from seconds to hours

(cherry picked from commit e4e97a3)
ValarDragon pushed a commit that referenced this pull request Feb 14, 2022
* min duration from 1d to 24h (d not recognized), changed weekly to day (weekly doesnt work, only week)

* changed from seconds to hours

(cherry picked from commit e4e97a3)

Co-authored-by: Adam Tucker <[email protected]>
@github-actions github-actions bot mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v6.x backport patches to v6.x branch
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants