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

Update ERC-6909: Move to Review #146

Closed
wants to merge 9 commits into from
Closed

Update ERC-6909: Move to Review #146

wants to merge 9 commits into from

Conversation

jtriley-eth
Copy link
Contributor

No description provided.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Dec 6, 2023

File ERCS/erc-6909.md

Requires 1 more reviewers from @axic, @g11tech, @Pandapip1, @SamWilsn, @xinbenlv

Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

  • Your abstract is a bit too terse. It should include a high-level technical description of how it solves the problem. In this case, maybe you could include a few sentences describing what it removes/adds compared to ERC-1155?
  • The interface ID shouldn't be in the rationale section. It belongs in the Specification section.
  • Same goes for the metadata extension. Since you're introducing new functionality, and not just explaining the reasoning behind your specification, it has to go in the specification section.

@jtriley-eth
Copy link
Contributor Author

jtriley-eth commented Dec 29, 2023

diff isn't showing bc i merged a different branch into main first, but recommended fixes are done

Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

You have a "SHOULD NOT" and a "MAY" requirement in your rationale section. All requirements should be in the specification section.

Copy link

github-actions bot commented Feb 6, 2024

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot removed the w-stale label Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants