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.

Sign up for GitHub

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

Jump to bottom

Update EIP-721: delete broken openzeppelin links #5903

Closed
wants to merge 2 commits into from
+0 −1
Closed

Update EIP-721: delete broken openzeppelin links #5903

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed500a5
docs: ERC721 implemention link correction
konojunya Nov 9, 2022
febd87d
fix: delete openzeppelin link
konojunya Nov 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion EIPS/eip-721.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ XXXXERC721, by William Entriken -- a scalable example implementation
1. Curio Cards. https://mycuriocards.com
1. Rare Pepe. https://rarepepewallet.com
1. Auctionhouse Asset Interface. https://github.com/dob/auctionhouse/blob/master/contracts/Asset.sol
1. OpenZeppelin SafeERC20.sol Implementation. https://github.com/OpenZeppelin/zeppelin-solidity/blob/master/contracts/token/ERC20/SafeERC20.sol
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the contribution!
This is a historical doc, to fix it

  1. Editors are working on a link policy per EIP-5757 (Add EIP-5757: Propose process for allowing external links #5757 ), let's wait for that to be finalized
  2. Links are generally suggested to use permlink to reduce chance of such broken links in the future

But still, thank you for your contribution! Greatly appreciated

Copy link
Author

Choose a reason for hiding this comment

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

@xinbenlv Yes. Exactly. This EIP is referenced by many people and we should wait for the Editors reply as it is a change to a part of that document.
I too recommend using permlink, but since the source of the change points to the default branch, I made the change using the same policy.

Copy link
Contributor

Choose a reason for hiding this comment

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

Links are generally suggested to use permlink to reduce chance of such broken links in the future

So, in this case, would the better fix be to replace the broken link with a permlink to the last SafeERC20, rather than a link to ERC721?

And, given these links are no normative, and this one is not discussed in the proposal, I'd be happy to see it removed.

1. OpenZeppelin ERC721.sol Implementation. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol

## Copyright

Expand Down