Put OEP templates in top level to make them easier to find #382
Conversation
There was a problem hiding this comment.
Hey Sarina, I think this is a good idea. I wasn't aware there were templates, and these would have helped.
It does seem like there are some warnings related to the toctrees:
/home/docs/checkouts/readthedocs.org/user_builds/open-edx-proposals/checkouts/382/oeps/processes/oep-0001.rst:448: WARNING: toctree contains reference to nonexisting document 'processes/oep-templates/pep-based-template'
/home/docs/checkouts/readthedocs.org/user_builds/open-edx-proposals/checkouts/382/oeps/processes/oep-0001.rst:448: WARNING: toctree contains reference to nonexisting document 'processes/oep-templates/adr-based-template'
/home/docs/checkouts/readthedocs.org/user_builds/open-edx-proposals/checkouts/382/oeps/processes/oep-0001.rst:448: WARNING: toctree contains reference to nonexisting document 'processes/oep-templates/external-link-template'
looking for now-outdated files... none found
|
@Carlos-Muniz thanks! I was going to hold on resolving errors until I get feedback on the substance of this change. |
|
substance makes sense to me! |
|
ugh, I can't provide links in the oeps/processes/.rst docs that link to the oep-templates/.rst docs because they're referenced in OEP-1 (and I think they should be) but that means they must be in OEP-1's directory structure - can't do relative linking (sphinx-doc/sphinx#701) some other solutions:
Any thoughts/other ideas? I tried doing a symbolic link as a few others have suggested/done in other projects, but it did not get the RTD build to pass. |
|
I would say it doesn't make sense to render the templates anyway: do authors need to see an HTML page entitled "OEP-XXXX: OEP Template"? They want to know how to get started making a .rst file of their own, so let's just link to the GitHub file. |
sarina
force-pushed
the
sarina/move-templates
branch
from
a8c3966 to
2aa62e6
Compare
|
Thanks @nedbat that's what I was leaning towards. I've made the change, I hope :) |
|
|
||
| .. | ||
| Add a hidden toctree to indicate that these files are referenced from here in the overall | ||
| table of contents. |
There was a problem hiding this comment.
As far as I can tell this never worked - https://open-edx-proposals.readthedocs.io/en/latest/processes/oep-0001.html - I don't see this in the table of contents at the top or the sub-section TOC here: https://open-edx-proposals.readthedocs.io/en/latest/processes/oep-0001.html#oep-structure-and-content
| :hidden: | ||
| .. _PEP-based template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/pep-based-template.rst | ||
| .. _ADR-based template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/adr-based-template.rst | ||
| .. _External link template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/external-link-template.rst |
There was a problem hiding this comment.
these links won't work until this merges
There was a problem hiding this comment.
Why make these external links? You should refer to the new location directly so that we can detect broken links as a part of the build process in the future: https://docs.openedx.org/en/latest/documentors/references/quick_reference_rst.html#links-between-rst-documents
There was a problem hiding this comment.
I have written in this PR how linking outside the doc tree doesn't work. #382 (comment)
If you can provide a different solution I am all ears. If you could provide a suggestion via the github UI that would be great because I could not find a solution.
There was a problem hiding this comment.
I thought the external links were a good solution, since they are meant to be used as .rst files, not as published HTML rendered from .rst files. The links-between-rst-documents will render the templates, which isn't helpful.
There was a problem hiding this comment.
Ah, makes sense, I think I would have gone with the literalinclude directive for this but I don't feel strongly enough about that to hold this up.
sarina
force-pushed
the
sarina/move-templates
branch
from
2aa62e6 to
990a1b0
Compare
| :hidden: | ||
| .. _PEP-based template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/pep-based-template.rst | ||
| .. _ADR-based template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/adr-based-template.rst | ||
| .. _External link template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/external-link-template.rst |
There was a problem hiding this comment.
Why make these external links? You should refer to the new location directly so that we can detect broken links as a part of the build process in the future: https://docs.openedx.org/en/latest/documentors/references/quick_reference_rst.html#links-between-rst-documents
| :hidden: | ||
| .. _PEP-based template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/pep-based-template.rst | ||
| .. _ADR-based template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/adr-based-template.rst | ||
| .. _External link template: https://github.com/openedx/open-edx-proposals/tree/master/oep-templates/external-link-template.rst |
There was a problem hiding this comment.
Ah, makes sense, I think I would have gone with the literalinclude directive for this but I don't feel strongly enough about that to hold this up.
sarina
force-pushed
the
sarina/move-templates
branch
from
990a1b0 to
8169c6e
Compare
When I was making my first OEP, I did not use a template as I did not see them buried in the processes directory. I propost to make them top-level to make them easier to find. I would also be ok with putting them into the oeps/ directory. Thoughts? @nedbat @feanil