-
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
refactor: Move to x/tx package #14634
Conversation
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.
This needs to be updated https://github.com/cosmos/cosmos-sdk/blob/94c2aff/.github/workflows/test.yml#L562-L590.
Could you go through this list: #14221?
Is there any reason why not do this now? Unless they depend on the SDK...? |
Yeah, quite heavily. I'll try to untangle those in follow-ups. |
@julienrbrt I think the checklist is done now, see c67abd0 |
@@ -1,4 +1,4 @@ | |||
module cosmossdk.io/tx | |||
module cosmossdk.io/x/tx |
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.
A PR adding a new vanity url must be created here: https://github.com/cosmos/vanity.
We should possibly bundle the addition of x/nft
and x/circuit
there too.
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.
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!
One tiny nit: changing the path here too https://github.com/cosmos/cosmos-sdk/blob/94c2aff/go.work.example#L13 |
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
Description
ref: #14600
Simply move the contents of
tx/
tox/tx
.In follow-ups, whenever it's useful we should:
types/tx
andx/auth/tx
tox/tx
when interesting, and when they don't depend on the root sdkAuthor Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change