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

Add Github-Only Comments to point users to ReadTheDocs #247

Closed
dongahn opened this issue Jun 5, 2020 · 7 comments · Fixed by #249
Closed

Add Github-Only Comments to point users to ReadTheDocs #247

dongahn opened this issue Jun 5, 2020 · 7 comments · Fixed by #249

Comments

@dongahn
Copy link
Member

dongahn commented Jun 5, 2020

I was browsing some of RFCs and it appears I can no longer follow the links using my Safari web browser. Example: trying to go to RFC12 from within RFC 16.

Screen Shot 2020-06-05 at 10 45 45 AM

Is this happening after we convert our RFC to the RST format?

@garlick
Copy link
Member

garlick commented Jun 5, 2020

They do work if access via this interface:

https://flux-framework.readthedocs.io/projects/flux-rfc/en/latest/

@dongahn
Copy link
Member Author

dongahn commented Jun 5, 2020

Yeah, that's much better and IT IS a beauty!

I don't know if it would also be straightforward to make this raw RFC interface also browsable to some extent. There will be developers who just get to the raw RFCs repo.

@gonsie or @SteVwonder: any idea?

As is, I can't even jump through the hyperlinks via our README files. Clicking on "in the Index" takes me to a page that I can't follow links any longer. My guess is it would probably be GitHub RST render so I don't know if there would be an easy fix:

Screen Shot 2020-06-05 at 11 04 33 AM

Screen Shot 2020-06-05 at 11 04 43 AM

@SteVwonder
Copy link
Member

The internal links within the RFCs use the :doc: role provided by Sphinx [1]. Rather unfortunately, GitHub's markup renderer does not support most Sphinx extensions to ReST, including :doc: [2]. So if we wanted these to work via the GitHub renderer, we would have to use explicit labels for each link, and those labels would have to be unique for every single link in the entire project [3]. That unfortunately wouldn't solve the problem for the other ReST features that GitHub does not support, including image width (#233) and includes (github/markup#172)

My suggestion would be to follow the advice at the top of GitHub's markup renderer [4]:

#  - If github is NOT the preferred viewer for the rst files (e.g.
#    the files are designed to be used with sphinx extensions at readthedocs)
#    then include a restructuredText comment at the top of the file
#    explaining that the document will display much more nicely with its
#    preferred viewer, e.g. at readthedocs.

We could add text (and a link) to the top of every file (including the Markdown readme) that would only show when rendered on GitHub that points people to the readthedocs site. The magic string to add to the start of the comment so that it only shows on GitHub is github display.

@SteVwonder
Copy link
Member

As is, I can't even jump through the hyperlinks via our README files. Clicking on "in the Index" takes me to a page that I can't follow links any longer.

The README that is rendered on the "homepage" of this repo is the README.md, so that actually should have working links (they work for me at least). The in the index link that you references actually points to the README.rst, which has the :doc: links that are broken on GitHub. So at least if someone lands on this repo and clicks on a link they see on the "homepage", it should work.

@dongahn
Copy link
Member Author

dongahn commented Jun 9, 2020

in the index link works but not these links in the destination page. You suggestion above makes sense to me.

@SteVwonder SteVwonder changed the title Hyperlinks within RFCs don't work Add Github-Only Comments to point users to ReadTheDocs Jun 11, 2020
@gonsie
Copy link
Contributor

gonsie commented Jun 18, 2020

@SteVwonder can you explain the github display thing? I’m happy to add the comments, but I’m not sure how to get it work.

@gonsie
Copy link
Contributor

gonsie commented Jun 18, 2020

nvm, I got it.

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 a pull request may close this issue.

4 participants