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

Implement Readthedocs #1423

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Implement Readthedocs #1423

wants to merge 2 commits into from

Conversation

Snell1224
Copy link
Collaborator

Including a folder that contains all the specific RTD files and configurations for LDMS documentation.

Copy link
Collaborator

@morrone morrone left a comment

Choose a reason for hiding this comment

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

I am not a huge fan of having all of those rst versions of the man pages statically committed into the repo. Those should really be auto-generated on demand so they are always up to date.

@bschwal
Copy link
Collaborator

bschwal commented Aug 20, 2024

Chris do you have a proposed mechanism for doing that auto generation? I think the idea was since the RTD is going to be versioned alongside the ldms/sos/etc repos that the static rst's would match the static version the RTD referenced. We intend, I believe, to generate new RTD versions and rst's with each tag version release. @Snell1224 is that what you intended too? @tom95858 any thoughts on versioning here?

@Snell1224
Copy link
Collaborator Author

@Snell1224 is that what you intended too?

Yes, the idea is to have a set of rst files for each version release so users can use the correct documentation depending on the LDMS version they have installed.

@tom95858
Copy link
Collaborator

I don't think this is @morrone's point. The issue is that (I think) the .rst files are generated by some human based on some other source file and that this allows for the original source and the read-the-docs to become out of sync. So specifically, how are these .rst files being generated? Directly by some human, indirectly by some tool, and do we have two files that have to be independently updated?

@bschwal
Copy link
Collaborator

bschwal commented Aug 21, 2024

These rst files were generated by a pandoc script I created which takes all man pages and creates rst files. We should include that script somewhere but I don't know a good landing spot.

@tom95858
Copy link
Collaborator

Thanks for the update @bschwal, please note that one of the merge checks failed. Specifically Space Check.

@tom95858
Copy link
Collaborator

Back to the point, if these files are generated by pandoc, why are they checked in?


.. code-block:: RST

> vi paper.lock
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not clear to me that this paper.lock stuff is really appropriate for a git repository.

@morrone
Copy link
Collaborator

morrone commented Aug 22, 2024

Chris do you have a proposed mechanism for doing that auto generation? I think the idea was since the RTD is going to be versioned alongside the ldms/sos/etc repos that the static rst's would match the static version the RTD referenced. We intend, I believe, to generate new RTD versions and rst's with each tag version release.

Yes. The rtd files should be generated from Makefile.am file(s) in much the same way that binaries are generated from source code. Write a rule that uses the conversion script (which either needs to be in-tree or a configure-time dependency) to "compile" the rtd files.

The problem with the current approach is that it works counter to the goal of ensuring that the rtd documents match the release. This approach makes it very easy for a release manager to forget to poke the people managing readthedocs to do a PR with refreshed documents before every tag. That would result in documents that do not match the ldms release. Automation means they are always in sync.

I imagine that we could even get fancy at some point and write github actions to automatically update readthedocs on each release tag.

And actually, this kind of raises an even bigger issue. None of this appears to be integrated into the ldms source code's build system. This shouldn't really land in the ldms source tree unless it is all properly integrated into the ldms source build system.

P.S. I'm not a huge fan of zip files in the source tree either.

@morrone
Copy link
Collaborator

morrone commented Aug 22, 2024

Also, the rtd/update_branches.py needs to be removed. I don't entirely understand the thinking there, but this process would violate our normal PR process. I do not think that we should be enabling that.

(edit: script name corrected)

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.

4 participants