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 ERC20 Guide link (was 404) #1799

Closed
wants to merge 1 commit into from

Conversation

wanderingstan
Copy link

@wanderingstan wanderingstan commented Jun 17, 2019

Link for "ERC20 Guide" on Token Readme is giving 404.

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/tokens.md#constructing-a-nice-erc20-token

Looks like tokens.md has been moved to the ./docs dir. The # link matched the section title so I assume this is where it was meant to go.

EDIT: No issue found for this. #1790 is also about broken links, but not this particular one.

@nventuro nventuro requested a review from frangio June 19, 2019 00:07
@frangio
Copy link
Contributor

frangio commented Jun 22, 2019

Hm, unfortunately the problem here is that the link is meant to work in the published site, where it does currently work, and not in the GitHub preview.

Merging this PR would break the published site, so I'll close it. But I understand this is not ideal. I'm thinking perhaps the documentation generator that parses these files should adjust the links so they work in both places.

@frangio frangio closed this Jun 22, 2019
@wanderingstan
Copy link
Author

Oh, I didn't even realize there was a "real" published site and that you're treating the repo as merely a preview.

Perhaps a stop-gap solution would be to put a standard note at the top of the all repo readme's saying something like "Please view our full documentation at https://docs.openzeppelin.org/"? This line could then be easily stripped by the doc generator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants