Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ReadTheDocs Configuration File #512

Merged
merged 14 commits into from
Mar 15, 2024
Merged

ReadTheDocs Configuration File #512

merged 14 commits into from
Mar 15, 2024

Conversation

amandarichardsonn
Copy link
Contributor

Adding readthedocs yaml file to develop

@amandarichardsonn amandarichardsonn changed the title pushing 1 file ReadTheDocs Configuration File Mar 6, 2024
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.75%. Comparing base (9e74ba9) to head (d7e763a).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #512   +/-   ##
========================================
  Coverage    90.75%   90.75%           
========================================
  Files           60       60           
  Lines         3839     3839           
========================================
  Hits          3484     3484           
  Misses         355      355           

@al-rigazzi al-rigazzi removed their assignment Mar 6, 2024
@al-rigazzi al-rigazzi requested a review from ashao March 6, 2024 19:19
Copy link
Collaborator

@ashao ashao left a comment

Choose a reason for hiding this comment

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

One comment on the robots.txt work

Another item to take care of requires configuring ReadTheDocs:

doc/conf.py Outdated
@@ -82,6 +82,8 @@
# a list of builtin themes.
html_theme = "sphinx_book_theme"

# Avoid indexing by search engines
html_extra_path = ['../robots.txt']
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will create a robots.txt file for all documentation that we build (including the one that will be hosted at craylabs.org. I was digging into the documentation a little more (and verified) that readthedocs by default ships a robots.txt: https://docs.readthedocs.io/en/stable/guides/technical-docs-seo-guide.html#seo-robots-txt

This seems to be the case for us, since this file exists: https://smartsim.readthedocs.io/en/latest/robots.txt

Could you dig into this a little more? It might also (for safety) be worthwhile to still figure out a way to make a robots.txt file but have a way to only create the robots.txt file if we know that it's being built via readthedocs

Could you dig into this and

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for sure!

@amandarichardsonn
Copy link
Contributor Author

amandarichardsonn commented Mar 13, 2024

@ashao

I removed the robots.txt file from the root directory.

It is now created in conf.py - read the docs provides the env variable READTHEDOCS (read more here). I am checking if the environment var is true (I could just check if it exists in the build env?). Then I create the robots.txt file that overwrites the one that read the docs creates. Let me know your thoughts on your next review!

@amandarichardsonn
Copy link
Contributor Author

amandarichardsonn commented Mar 14, 2024

Good catch to add the canonical URL! The docs were super clear to why we need this. You can check the add here (Details).

@amandarichardsonn amandarichardsonn requested a review from ashao March 14, 2024 16:48
Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

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

Approving with only minor comments and pending @ashao review

Copy link
Collaborator

@ashao ashao left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this! Some very minor comments that shouldn't hold up merging.

In addition to the make docs update ticket that I suggest, I'm going to open up another one about the canonical URL. What it looks like that means is actually that we could update craylabs.org to point to ReadTheDocs instead. That's not happening now, because we don't have our domain provider set up like that, but I think we should think through whether we want to do that.

Comment on lines +22 to +32
pre_create_environment:
- git clone --depth 1 https://github.com/CrayLabs/SmartRedis.git smartredis
- git clone --depth 1 https://github.com/CrayLabs/SmartDashboard.git smartdashboard
post_create_environment:
- python -m pip install .
- cd smartredis; python -m pip install .
- cd smartredis/doc; doxygen Doxyfile_c; doxygen Doxyfile_cpp; doxygen Doxyfile_fortran
- ln -s smartredis/examples ./examples
- cd smartdashboard; python -m pip install .
pre_build:
- python -m sphinx -b linkcheck doc/ $READTHEDOCS_OUTPUT/linkcheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you make a ticket to update the make docs target so that we can eventually converge the two workflows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes!

@@ -84,6 +84,13 @@
# a list of builtin themes.
html_theme = "sphinx_book_theme"

# Check if the environment variable is set to 'True'
if os.environ.get('READTHEDOCS') == "True":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice solution!

@amandarichardsonn amandarichardsonn added area: CI/CD Issues related to continuous integration and deployment area: docs Issues related to documentation type: feature Issues that include feature request or feature idea labels Mar 15, 2024
@amandarichardsonn amandarichardsonn merged commit 10e084e into develop Mar 15, 2024
43 checks passed
@amandarichardsonn amandarichardsonn deleted the read-yaml branch March 15, 2024 22:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area: CI/CD Issues related to continuous integration and deployment area: docs Issues related to documentation non-user-facing type: feature Issues that include feature request or feature idea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants