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

Remove duplicate denom-authority-metadata query command #4819

Merged
merged 9 commits into from
May 30, 2023

Conversation

phamminh0811
Copy link
Contributor

Closes: #4784

Brief Changelog

Remove duplicate "denom-authority-metadata" query command

@phamminh0811 phamminh0811 added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Apr 3, 2023
Copy link
Member

@pysel pysel left a comment

Choose a reason for hiding this comment

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

LGTM, thanks, but could you please add labels to backport this to v15

@phamminh0811 phamminh0811 added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels Apr 4, 2023
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@github-actions github-actions bot added the Stale label Apr 19, 2023
@pysel pysel removed the Stale label Apr 19, 2023
@pysel pysel requested review from p0mvn and mattverse April 19, 2023 00:29
@byeongsu-hong
Copy link
Contributor

How about change the command handler to GetCmdDenomsFromCreator?

Comment on lines 22 to +23
osmocli.AddQueryCmd(cmd, types.NewQueryClient, GetCmdDenomAuthorityMetadata)
osmocli.AddQueryCmd(cmd, types.NewQueryClient, GetCmdDenomAuthorityMetadata)
osmocli.AddQueryCmd(cmd, types.NewQueryClient, GetCmdDenomsFromCreator)
Copy link
Member

@czarcas7ic czarcas7ic May 7, 2023

Choose a reason for hiding this comment

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

@phamminh0811 can you please do two things:

  1. Add the GetCmdDenomBeforeSendHook query here as well
  2. Test them locally on mainnet and ack that they behave as expected

As soon as thats done, please ping me and I will merge :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, please check

@czarcas7ic
Copy link
Member

@phamminh0811 any update on this?

@p0mvn
Copy link
Member

p0mvn commented May 29, 2023

@phamminh0811 @pysel any updates here?

@p0mvn p0mvn merged commit 50b26a8 into osmosis-labs:main May 30, 2023
1 check passed
mergify bot pushed a commit that referenced this pull request May 30, 2023
* remove duplicate denom-authority-metadata query command

* merge main

* revert change

* revert change

* fix lint

* add GetCmdDenomsFromCreator and CHANGELOG

---------

Co-authored-by: Jacob Gadikian <[email protected]>
(cherry picked from commit 50b26a8)

# Conflicts:
#	CHANGELOG.md
pysel pushed a commit that referenced this pull request Jun 6, 2023
* remove duplicate denom-authority-metadata query command

* merge main

* revert change

* revert change

* fix lint

* add GetCmdDenomsFromCreator and CHANGELOG

---------

Co-authored-by: Jacob Gadikian <[email protected]>
@github-actions github-actions bot mentioned this pull request Feb 1, 2024
@github-actions github-actions bot mentioned this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v15.x backport patches to v15.x branch C:CLI C:x/tokenfactory V:state/compatible/backport State machine compatible PR, should be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate denom-authority-metadata query command
6 participants