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 EIP-721: Add ERC links #7550

Closed
wants to merge 9 commits into from
Closed

Conversation

fulldecent
Copy link
Contributor

This fixes some linter complaints

Here is a validation run that reporting the error which this PR fixes.

https://github.com/ethereum/EIPs/actions/runs/6045138001/job/16404727259?pr=6527

@github-actions github-actions bot added c-update Modifies an existing proposal s-final This EIP is Final t-erc labels Sep 1, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Sep 1, 2023

File EIPS/eip-721.md

Requires 1 more reviewers from @axic, @gcolvin, @lightclient, @SamWilsn

@eth-bot eth-bot changed the title Add ERC links Update EIP-721: Add ERC links Sep 1, 2023
@eth-bot eth-bot added the e-consensus Waiting on editor consensus label Sep 1, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Sep 1, 2023
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

ERC-721 should be replaced with "This EIP" instead of self-referencing by number.

Library links aren't supported. Use relative links instead (i.e. [ERC-20](./eip-20.md) instead of [ERC-20](eip-20).

@fulldecent
Copy link
Contributor Author

fulldecent commented Sep 1, 2023

@Pandapip1 Thank you for your notes. I have updated all references to "ERC-721" from within that document to "this ERC". Some of this required a little disambiguation because this document is referring to an ERC (numbered 721) an interface (named ERC721) and two other interfaces (ERC721Enumerable, ERC721Metadata). My action here was to maintain clarity about what was being discussed while minimizing word changes.

This document previously refers to these things as "ERC"s. I kept this nomenclature in the word changes above. If there will consideration of changing this to "EIP", please let's separate that to a different, independent discussion and PR.

A reference was previously made to 0xcert's implementation which is named ERC-721. I updated this to its new name "Nibbstack". But the new product is still named ERC721. Probably this will also trigger a false-positive lint.

EIPS/eip-721.md Outdated Show resolved Hide resolved
Pandapip1
Pandapip1 previously approved these changes Sep 1, 2023
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

+1 if my change fixed the errors

@fulldecent
Copy link
Contributor Author

@Pandapip1 thank you, yes that's good now!

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Sep 2, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Sep 2, 2023
Pandapip1
Pandapip1 previously approved these changes Sep 5, 2023
@Pandapip1 Pandapip1 added this to the Manual Merge Queue milestone Sep 5, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Sep 5, 2023
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.

I undid a few content changes, and fixed a few more eipw errors. Still a bunch of them because of the external links.

I'll add this to tomorrow's EIPIP call agenda.

@SamWilsn SamWilsn mentioned this pull request Sep 5, 2023
8 tasks
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

The commit 7e2b87e (as a parent of aecf612) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Sep 5, 2023
@bumblefudge
Copy link
Contributor

thanks for this

@SamWilsn
Copy link
Contributor

I am closing this pull request because we are in the process of separating EIPs and ERCs into distinct repositories. Unfortunately, as far as we are aware, GitHub does not provide any tools to ease this migration, so every pull request will need to be re-opened manually.

As this is a PR to create / modify an ERC, I will kindly ask you to redirect this to the new repository at ethereum/ERCs. We have prepared a guide to help with the process.

If there is relevant history here, please link to this PR from the new pull request.

On behalf of the EIP Editors, I apologize for this inconvenience.

@SamWilsn SamWilsn closed this Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal e-consensus Waiting on editor consensus s-final This EIP is Final t-erc w-ci Waiting on CI to pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants