-
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
feat: use x/nft as a standalone module #10578
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.
Will this be 1.0 from the first release? Also should it target 0.44?
There is a problem with this PR temporarily, because after nft becomes an independent project, the simapp in cosmos-sdk cannot be used directly, so the test code cannot pass now. Is there any good solution? |
you can recreate a minimal simp for simulations, this is until we have better a testing design . |
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. How about the sim tests?
I see two approvals but I don't see how this PR can work because it depends on simapp still and simulations with x/nft won't actually run. |
I think the idea is what @marbar3778 suggested:
In both cases we need need to double check the CI if the tests are running. |
👌🏻,i got it |
@@ -0,0 +1,600 @@ | |||
package simapp | |||
|
|||
import ( |
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.
thanks for adding this, any chance you can get rid of all modules except what is needed? this will help with reducing the changes needed in this file in the future.
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 PR should probably be closed because someone is doing the same thing
closing this for now based of comment above |
Description
Closes: #9826
Author 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