Skip to content

build: update docs build #2339

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

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

build: update docs build #2339

wants to merge 2 commits into from

Conversation

parthea
Copy link
Contributor

@parthea parthea commented Feb 18, 2025

Towards #2338

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Feb 18, 2025
@parthea parthea requested review from vchudnov-g and ohmayr February 19, 2025 14:53
@parthea parthea marked this pull request as ready for review February 19, 2025 14:53
@parthea parthea requested a review from a team as a code owner February 19, 2025 14:53
--github-repository=$(jq --raw-output '.repo // empty' .repo-metadata.json) \
--issue-tracker=$(jq --raw-output '.issue_tracker // empty' .repo-metadata.json)

cat docs.metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

cruft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be fixed in cl/742192972

# create metadata
python3 -m docuploader create-metadata \
python3.10 -m docuploader create-metadata \
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is docuploader being installed now? I see it uses to be pip installed in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This script was moved internally and is no longer included in this PR.

noxfile.py Outdated
@@ -644,7 +644,9 @@ def snippetgen(session):

@nox.session(python="3.10")
def docs(session):
"""Build the docs."""
"""Build the docs for this library."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just for the generator, right? We could reword to minimize confusion.

Suggested change
"""Build the docs for this library."""
"""Build the docs for this generator."""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d0eaba7

@parthea parthea force-pushed the update-docs-build branch from e011566 to 9837f1d Compare March 23, 2025 10:22
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

Two cosmetic suggestions and a minor question.

Comment on lines +672 to +677
"-b",
"html",
"-d",
os.path.join("docs", "_build", "doctrees", ""),
os.path.join("docs", ""),
os.path.join("docs", "_build", "html", ""),
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for documenting the args above! I suggest document the following, and also putting arg values on the same line as their args:

Suggested change
"-b",
"html",
"-d",
os.path.join("docs", "_build", "doctrees", ""),
os.path.join("docs", ""),
os.path.join("docs", "_build", "html", ""),
"-b", "html", # builder
"-d", os.path.join("docs", "_build", "doctrees", ""), # cache directory
# paths to build:
os.path.join("docs", ""),
os.path.join("docs", "_build", "html", ""),

I'm basing this interpretation of the args on https://www.sphinx-doc.org/en/master/man/sphinx-build.html

"sphinx-build",
"-T", # show full traceback on exception
"-N", # no colors
"-D",
Copy link
Contributor

Choose a reason for hiding this comment

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

As in the previous comment, explain more of the args

@@ -86,7 +86,7 @@
# The theme to use for HTML and HTML Help pages. See the documentation for
# a list of builtin themes.
#
html_theme = "sphinx_rtd_theme"
html_theme = "alabaster"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious on why the theme change? I think I do prefer "alabaster" to "rtd", but I think the paramount issue is being consistent throughout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants