-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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-1: Fix outdated links #5706
Conversation
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s): (fail) eip-1.md
|
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.
LGTM other than suggest using link to a specific commit when possible.
@xinbenlv, I didn't quite understand what you meant like this?
3c43b2b
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.
For the sake of not dragging this out, you don't have to make @xinbenlv's change. It would certainly be preferred, but it's probably best to just stop linking to nonexistent pages in the first place.
@Pandapip1 personally, I feel these links being touched are helpful. IMHO Updating the links as opposed to removing the links is great. The links exist to give references to the content and I think they are all proper links. (cc: @gcolvin @lightclient) @donBarbos here is a little context that editors are debating whether linking to external links shall be allowed or to what extent. See #5597. You might (and IMHO, are encouraged to) make your judgement about whether links are helpful and should continue to exist in EIP-1. |
I was agreeing. I was just saying that a speedy merge was preferable to not making any changes. |
* Update eip-1.md * Update eip-1.md @xinbenlv, I didn't quite understand what you meant like this?
updated invalid links to new paths