ROCm / rocm-cmake

CMake modules used within the ROCm libraries
https://rocm.docs.amd.com/projects/ROCmCMakeBuildTools/en/latest/
MIT License
62 stars 43 forks source link

Update documentation to match ROCm #142

Closed lawruble13 closed 9 months ago

lawruble13 commented 1 year ago

The rocm-cmake documentation should be updated to match the documentation in the rest of ROCm, and be included on rocm.docs.amd.com.

lawruble13 commented 1 year ago

One thing is that it will need an RTD config.

Thanks! Couldn't remember how to specify the build configuration.

For the conf.py, will likely want to call the ROCmDocs constructor (https://github.com/RadeonOpenCompute/rocm-docs-core#use)

Noting in advance that I wrote that particular usage guideline, I'm actually of the opinion that we should transition to using rocm-docs-core as the Sphinx extension/theme combination that it is rather than an object that we inject into the global scope. That standard usage guideline does almost exactly what I've done here, although there is a small amount of logic surrounding the use of Doxygen. Given that this project doesn't use doxygen, this is equivalent to the usage recommendation in rocm-docs-core.

cgmb commented 1 year ago

I'm actually of the opinion that we should transition to using rocm-docs-core as the Sphinx extension/theme combination that it is rather than an object that we inject into the global scope.

+1

samjwu commented 1 year ago

RTD Builds for PRs have been enabled for this project. They should appear in the GitHub commit status and checks the next time this PR is updated. If not, closing and reopening the PR has worked in the past for triggering PR doc builds.

saadrahim commented 10 months ago

@lawruble13 can this be merged soon? We need documentation for rocm-cmake soon.

samjwu commented 10 months ago

https://github.com/lawruble13/rocm-cmake/pull/3 adds the yaml configuration for building on ReadtheDocs as well as some other changes from https://github.com/RadeonOpenCompute/rocm-docs-core/issues/330

pfultz2 commented 10 months ago

I think my only concern is that the src directory name ends up in the user-visible URL. If we can mitigate that, this is perfect.

To do that we would need to move the conf.py to the src directory, I believe. I dont know if thats an issue or not. @samjwu Is there an issue with moving this file as well?

samjwu commented 10 months ago

I think my only concern is that the src directory name ends up in the user-visible URL. If we can mitigate that, this is perfect.

To do that we would need to move the conf.py to the src directory, I believe. I dont know if thats an issue or not. @samjwu Is there an issue with moving this file as well?

It's preferable to keep a consistent doc structure among the projects, but it's possible to move it. I am not sure if what would break rocm_add_sphinx_doc though. @lawruble13 thoughts?

The external_toc_path and path to CMakeLists.txt in conf.py and sphinx.configuration in .readthedocs.yaml (and entries in _toc.yml.in) would have to be updated to match the new structure.

cgmb commented 10 months ago

It's a bit off-topic, but why are we using external_toc again? Does rocm-docs-core need to do something with it?

samjwu commented 9 months ago

It's a bit off-topic, but why are we using external_toc again? Does rocm-docs-core need to do something with it?

rocm-docs-core has it as a dependency.

The main advantage is ease of managing table of contents as they are defined in a single yaml file, compared to having toctrees in possibly multiple files

pfultz2 commented 9 months ago

compared to having toctrees in possibly multiple files

Toctrees seem easier to manage as they are automatically generated from the included files.