-
Notifications
You must be signed in to change notification settings - Fork 12.2k
Remove draft- prefix for ERC6909 #5929
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
Conversation
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR removes the “draft-” prefix from the ERC6909 implementation and its extensions to align with the EIP-6909 last call status by renaming files and updating header comments and import paths. Class diagram for updated ERC6909 implementation and extensionsclassDiagram
class ERC6909 {
}
class ERC6909ContentURI {
ERC6909ContentURI --> ERC6909
}
class ERC6909Metadata {
ERC6909Metadata --> ERC6909
}
class ERC6909TokenSupply {
ERC6909TokenSupply --> ERC6909
}
class ERC6909GameItems {
ERC6909GameItems --> ERC6909Metadata
GOLD : uint256
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
WalkthroughThis change updates ERC-6909 references from draft to final paths across several contracts. Header comments and import paths are revised in IERC6909.sol, ERC6909.sol, and extensions (ERC6909ContentURI.sol, ERC6909Metadata.sol, ERC6909TokenSupply.sol), as well as a mock (ERC6909GameItems.sol). References switch from draft-IERC6909.sol and draft-ERC6909*.sol to IERC6909.sol and ERC6909*.sol. No functional logic, control flow, or public API signatures are modified. Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (6)
contracts/interfaces/IERC6909.sol (1)
2-2
: Header path update looks correct; consider syncing the “last updated” version tag.This file shows v5.4.0 while sibling ERC6909 files still say v5.3.0. If this PR lands in the same release, align the version tag across touched files; otherwise, keep as-is per release process.
contracts/token/ERC6909/ERC6909.sol (2)
2-2
: Confirm version tag consistency before release.This file still says v5.3.0 while IERC6909 uses v5.4.0. Align during release bump if intended.
6-6
: Optional: add deprecation shims to ease downstream migration.Consider keeping thin forwarding files at old paths (e.g., token/ERC6909/extensions/draft-ERC6909Metadata.sol) that only import the new files, so existing importers don’t break immediately. You can remove them in the next major.
contracts/token/ERC6909/extensions/ERC6909Metadata.sol (1)
2-2
: Minor: check the “last updated” version tag.Keep consistent with the targeted release (v5.3.0 vs v5.4.0).
contracts/token/ERC6909/extensions/ERC6909ContentURI.sol (1)
2-2
: Version tag consistency.Same note as other files; align during release bump.
contracts/token/ERC6909/extensions/ERC6909TokenSupply.sol (1)
2-2
: Minor: sync “last updated” tag with the intended release.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
contracts/interfaces/IERC6909.sol
(1 hunks)contracts/mocks/docs/token/ERC6909/ERC6909GameItems.sol
(1 hunks)contracts/token/ERC6909/ERC6909.sol
(1 hunks)contracts/token/ERC6909/extensions/ERC6909ContentURI.sol
(1 hunks)contracts/token/ERC6909/extensions/ERC6909Metadata.sol
(1 hunks)contracts/token/ERC6909/extensions/ERC6909TokenSupply.sol
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: coverage
- GitHub Check: halmos
🔇 Additional comments (6)
contracts/token/ERC6909/ERC6909.sol (2)
6-6
: Import path switched to non-draft interface — LGTM.
2-2
: No draft-ERC6909 references found — verify and close.
Executed the provided rg/fd script; no matches were returned in the repo.contracts/token/ERC6909/extensions/ERC6909Metadata.sol (1)
6-7
: Updated imports to non-draft names — looks good.contracts/mocks/docs/token/ERC6909/ERC6909GameItems.sol (1)
4-4
: Mock import updated to new path — LGTM.contracts/token/ERC6909/extensions/ERC6909ContentURI.sol (1)
6-7
: Non-draft imports in place — LGTM.contracts/token/ERC6909/extensions/ERC6909TokenSupply.sol (1)
6-7
: Imports migrated from draft to final — looks correct.
Lets wait before it is actually final before we merge that. |
Note: In 4.9.0 we had the following changelog entry
I believe we should have a similar changelog entry. I'm also wondering if we should should not keep the old (draft) files as a redirect. |
ERC-6909 is now in Last Call: https://eips.ethereum.org/EIPS/eip-6909
PR Checklist
npx changeset add
)Summary by Sourcery
Remove the 'draft-' prefix from ERC6909 contracts and interfaces now that the EIP is in Last Call, and update all related filenames, headers, and imports accordingly.
Enhancements: