Skip to content

Conversation

@jtriley2p
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.

@jtriley2p
Copy link
Contributor Author

jtriley2p 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.

@github-actions
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
@jtriley2p jtriley2p closed this Feb 16, 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