chore: deprecate nft for future #24575
Conversation
Ironbird - launch a networkTo use Ironbird, you can use the following commands:
Custom Load Test ConfigurationYou can provide a custom load test configuration using the `--load-test-config=` flag:Use |
📝 WalkthroughWalkthroughThe changes introduce widespread deprecation notices and linter suppression comments throughout the codebase, specifically targeting the Changes
Sequence Diagram(s)No sequence diagram is generated as the changes are limited to documentation, comments, and linter suppression without affecting control flow or introducing new features. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (1)
39-39: Fix heading level for Deprecated section.
The heading "### Deprecated" should only increment one level from the previous H1 ("# Changelog"), so it should be "## Deprecated" to satisfy markdown lint rules.- ### Deprecated + ## Deprecated🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
39-39: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
x/nft/keeper/genesis.go (1)
6-6: Import-level deprecation suppression is correct
The//nolint:staticcheck // deprecated and to be removeddirective properly silences deprecation warnings forcosmossdk.io/x/nft.Consider adding a Go-style deprecation comment at the top of the file to make it explicit in documentation, for example:
+// Deprecated: The x/nft module is deprecated and will be removed in the next major release. +// Please migrate to github.com/cosmos/cosmos-sdk-legacy/x/nft. + package keeper
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
CHANGELOG.md(1 hunks)simapp/app.go(3 hunks)simapp/app_config.go(1 hunks)x/nft/README.md(1 hunks)x/nft/keeper/class.go(1 hunks)x/nft/keeper/genesis.go(1 hunks)x/nft/keeper/grpc_query.go(1 hunks)x/nft/keeper/grpc_query_test.go(1 hunks)x/nft/keeper/keeper.go(1 hunks)x/nft/keeper/keeper_test.go(1 hunks)x/nft/keeper/keys.go(1 hunks)x/nft/keeper/msg_server.go(1 hunks)x/nft/keeper/msg_server_test.go(1 hunks)x/nft/keeper/nft.go(1 hunks)x/nft/keeper/nft_batch.go(1 hunks)x/nft/keeper/nft_batch_test.go(1 hunks)x/nft/keys.go(1 hunks)x/nft/module/autocli.go(1 hunks)x/nft/module/module.go(1 hunks)x/nft/simulation/decoder.go(1 hunks)x/nft/simulation/decoder_test.go(1 hunks)x/nft/simulation/genesis.go(1 hunks)x/nft/simulation/genesis_test.go(1 hunks)x/nft/simulation/msg_factory.go(1 hunks)x/nft/simulation/operations.go(1 hunks)x/nft/simulation/operations_test.go(1 hunks)x/nft/testutil/app_config.go(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
39-39: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: test-system-legacy
- GitHub Check: test-system
- GitHub Check: tests (03)
- GitHub Check: tests (02)
- GitHub Check: tests (01)
- GitHub Check: tests (00)
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-integration
- GitHub Check: build (arm64)
- GitHub Check: test-e2e
- GitHub Check: build (amd64)
- GitHub Check: Analyze
- GitHub Check: Check docs build
- GitHub Check: Gosec
- GitHub Check: golangci-lint
- GitHub Check: Summary
🔇 Additional comments (30)
CHANGELOG.md (1)
41-41: Deprecation entry for x/nft module is accurately documented.
The new entry correctly marks thex/nftmodule as deprecated, includes the PR link, and points to the legacy repo location.x/nft/simulation/genesis.go (1)
7-7: Deprecation suppression added to nft import.
The//nolint:staticcheckcomment on thecosmossdk.io/x/nftimport properly suppresses deprecation warnings and aligns with the planned removal of the module.x/nft/keeper/class.go (1)
8-8: Suppress staticcheck on deprecated nft import.
Adding//nolint:staticcheckto thecosmossdk.io/x/nftimport consistently suppresses linter warnings for the deprecated module.x/nft/keeper/nft_batch.go (1)
7-7: Suppress staticcheck on deprecated nft import.
The import directive forcosmossdk.io/x/nftnow includes//nolint:staticcheck, correctly marking the package as deprecated for batch operations as well.x/nft/keeper/nft.go (1)
8-8: Suppress staticcheck on deprecated nft import.
The//nolint:staticcheckannotation on thecosmossdk.io/x/nftimport ensures deprecation warnings are suppressed across core keeper logic.x/nft/keeper/keys.go (1)
6-6: Deprecation annotation on import is consistent
Using//nolint:staticcheck // deprecated and to be removedcorrectly suppresses the staticcheck warning for this deprecated import.Optionally, to improve clarity for future maintainers, you could add a file-level deprecation notice above the
packagedeclaration as shown for the other keeper files.x/nft/keeper/msg_server.go (1)
8-8: Suppressing linter warnings on deprecated import is appropriate
The added//nolint:staticcheck // deprecated and to be removedon thecosmossdk.io/x/nftimport correctly silences deprecation alerts without modifying logic.As with the other files, consider adding a brief file-header comment indicating the module’s deprecated status.
x/nft/simulation/msg_factory.go (1)
6-7: Deprecation suppression on both imports is correct
The dual//nolint:staticcheck // deprecated and to be removeddirectives oncosmossdk.io/x/nftand itskeepersubpackage imports cleanly suppress deprecation warnings.Optionally, a file-level deprecation prologue could reinforce that this simulation code is tied to a deprecated module.
x/nft/module/module.go (1)
16-18: Import annotations properly suppress staticcheck errors for deprecated modules
Markingcosmossdk.io/x/nft, itskeeper, andsimulationsubpackages with//nolint:staticcheck // deprecated and to be removedis consistent with the deprecation effort.You may optionally add a top-of-file deprecation notice to adhere to Go’s deprecation conventions, for example:
+// Deprecated: The x/nft module has been moved to github.com/cosmos/cosmos-sdk-legacy. +// All support for this path will be dropped in the next major release. + package modulex/nft/simulation/operations.go (1)
6-7: Appropriate deprecation annotations added.The annotations correctly mark the
x/nftandx/nft/keeperpackages as deprecated with the appropriate//nolint:staticcheckdirective, which will properly suppress linter warnings while clearly documenting the deprecation status.x/nft/keeper/nft_batch_test.go (1)
7-7: Appropriate deprecation annotation added.The annotation correctly marks the
x/nftpackage as deprecated with the appropriate//nolint:staticcheckdirective, which will properly suppress linter warnings while clearly documenting the deprecation status.x/nft/simulation/operations_test.go (1)
13-15: Appropriate deprecation annotations added.The annotations correctly mark the
x/nft,x/nft/keeper, andx/nft/simulationpackages as deprecated with the appropriate//nolint:staticcheckdirective, which will properly suppress linter warnings while clearly documenting the deprecation status.x/nft/keeper/grpc_query.go (1)
7-7: Appropriate deprecation annotation added.The annotation correctly marks the
x/nftpackage as deprecated with the appropriate//nolint:staticcheckdirective, which will properly suppress linter warnings while clearly documenting the deprecation status.x/nft/keeper/grpc_query_test.go (1)
11-11: Consistent deprecation annotation added.The addition of the
//nolint:staticcheckcomment to suppress warnings is appropriate as part of the broader deprecation strategy. This ensures the codebase can compile without linter errors while using the module that will eventually be removed.x/nft/keys.go (1)
1-1: Clear deprecation notice follows best practices.The deprecation comment clearly communicates that the package will be removed in the next major release and provides the future location of the code. This follows best practices by giving users advance notice and migration guidance.
x/nft/keeper/msg_server_test.go (1)
6-6: Consistent deprecation annotation added.The
//nolint:staticcheckcomment matches the approach used in other files, maintaining consistency throughout the codebase in how deprecated modules are marked.x/nft/README.md (1)
7-8: Prominent deprecation warning in documentation.The added warning is appropriately prominent with the warning emoji and clear messaging about the deprecation status. This ensures that developers consulting the documentation are immediately aware of the module's status.
x/nft/simulation/decoder_test.go (1)
9-12: Approve deprecation annotations on NFT importsThe added
//nolint:staticcheck // deprecated and to be removedcomments correctly suppress static analysis warnings for the deprecatedx/nftimports in this test file, aligning with the module deprecation plan.x/nft/testutil/app_config.go (1)
4-4: Approve blank import deprecation annotationThe
//nolint:staticcheck // deprecated and to be removeddirective appropriately marks the blank import ofx/nft/modulefor deprecation while preserving it for app wiring.x/nft/keeper/keeper_test.go (1)
12-14: Approve deprecation suppression on keeper test importsThe
//nolint:staticcheck // deprecated and to be removedannotations effectively silence deprecation warnings for thex/nft,x/nft/keeper, andx/nft/moduleimports in the keeper test suite.x/nft/simulation/genesis_test.go (1)
11-13: Approve deprecation suppression for simulation genesis test importsThese
//nolint:staticcheck // deprecated and to be removedcomments correctly flag thex/nftimports as deprecated and suppress staticcheck warnings without affecting test logic.x/nft/keeper/keeper.go (2)
1-1: Approve deprecation package commentThe file-level deprecation notice clearly communicates the removal timeline and relocation of the
x/nftmodule, which is essential for developers and maintainers.
7-7: Approve staticcheck suppression on NFT importThe
//nolint:staticcheck // deprecated and to be removedannotation on thex/nftimport is correctly applied to suppress deprecation warnings while the import remains necessary.simapp/app_config.go (1)
38-40: Ensure consistent deprecation lint suppression for NFT importsThe
cosmossdk.io/x/nftimport in this file lacks a//nolint:staticcheckdirective, unlike other parts of the codebase where deprecatedx/nftimports are suppressed. Consider adding//nolint:staticcheckto maintain uniform handling of the deprecated module across all import sites.x/nft/module/autocli.go (2)
1-1: Deprecation notice format is correctThe package-level
// Deprecated:comment properly follows Go conventions and clearly indicates the module's removal plan.
9-9: Suppress staticcheck warnings for deprecated NFT importAdding
//nolint:staticcheckto thecosmossdk.io/x/nftimport correctly silences linter warnings for this deprecated package.simapp/app.go (2)
31-33: Suppress staticcheck warnings for deprecated NFT importsThe
x/nft,x/nft/keeper, andx/nft/moduleimports are now annotated with//nolint:staticcheck, aligning with the deprecation policy across the application wiring.
470-470: Style-only comment adjustment: no action neededThis change converts a block comment to a line comment with adjusted indentation and does not affect functionality.
x/nft/simulation/decoder.go (2)
1-1: Deprecation notice format is correctThe package-level
// Deprecated:comment follows Go conventions for deprecation notices and clearly states the removal plan.
8-9: Suppress staticcheck warnings for deprecated simulation importsThe
cosmossdk.io/x/nftandcosmossdk.io/x/nft/keeperimports are annotated with//nolint:staticcheck, ensuring deprecated imports do not trigger linter warnings in simulation code.
Summary by CodeRabbit
Documentation
Style
Chores