gemini-code-assist[bot] commented on code in PR #343: URL: https://github.com/apache/tvm-ffi/pull/343#discussion_r2616155320
########## docs/README.md: ########## @@ -95,6 +95,18 @@ BUILD_CPP_DOCS=1 BUILD_RUST_DOCS=1 uv run --group docs sphinx-autobuild docs doc BUILD_CPP_DOCS=1 BUILD_RUST_DOCS=1 uv run --group docs sphinx-build -M html docs docs/_build ``` +### Multi-version build (main + tags) + +Build the documentation for `main` and tags matching `vX.Y.Z` (for example `v0.1.0`, `v0.1.5`). `sphinx-multiversion` builds from git archives, so the helper script sets a pretend version and regenerates the switcher JSON automatically: + +```bash +BUILD_CPP_DOCS=1 BUILD_RUST_DOCS=1 BASE_URL="/" uv run --group docs bash docs/build_multiversion.sh Review Comment:  The example command for the multi-version build could be made more robust and reproducible by specifying the Python version and using an isolated environment, as is good practice with `uv`. This ensures the build uses the correct dependencies and Python interpreter as defined in `pyproject.toml`, avoiding potential conflicts with the user's global environment. ```suggestion BUILD_CPP_DOCS=1 BUILD_RUST_DOCS=1 BASE_URL="/" uv run --isolated --python 3.13 --group docs bash docs/build_multiversion.sh ``` ########## docs/conf.py: ########## @@ -44,15 +48,33 @@ # -- General configuration ------------------------------------------------ # Determine version without reading pyproject.toml -# Always use setuptools_scm (assumed available in docs env) -__version__ = setuptools_scm.get_version(root="..") +# sphinx-multiversion builds from git archives (no .git), so allow a fallback +# using the version name that the extension injects into the environment. -project = "tvm-ffi" -author = "Apache TVM FFI contributors" +def _get_version() -> str: + env_version = ( + os.environ.get("SPHINX_MULTIVERSION_NAME") + or os.environ.get("SPHINX_MULTIVERSION_VERSION") + or os.environ.get("READTHEDOCS_VERSION") + ) + if env_version: + return env_version + + try: + return setuptools_scm.get_version(root="..", fallback_version="0.0.0") + except Exception: Review Comment:  Using a broad `except Exception:` can mask unexpected errors and make debugging more difficult. It's better to catch a more specific exception. `setuptools_scm.get_version` raises a `LookupError` when it cannot determine the version, so catching that specific error would be more precise and robust. ```suggestion except LookupError: ``` ########## docs/tools/write_versions_json.py: ########## @@ -0,0 +1,166 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Convert sphinx-multiversion metadata into the version switcher JSON. + +Usage: + python docs/tools/write_versions_json.py <html_root> [--base-url /] +""" + +from __future__ import annotations + +import argparse +import json +from datetime import datetime +from pathlib import Path + +from packaging import version as pkg_version + +ROOT_VERSION = "main" +DEFAULT_BASE_URL = "/" +METADATA_NAME = "versions_metadata.json" + + +def _parse_creatordate(raw: str) -> datetime: + try: + return datetime.strptime(raw, "%Y-%m-%d %H:%M:%S %z") + except Exception: Review Comment:  Catching a broad `Exception` can hide other potential issues. The `datetime.strptime` function specifically raises a `ValueError` when the date string does not match the format. It's better practice to catch this specific exception for more precise error handling. ```suggestion except ValueError: ``` ########## docs/tools/write_versions_json.py: ########## @@ -0,0 +1,166 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Convert sphinx-multiversion metadata into the version switcher JSON. + +Usage: + python docs/tools/write_versions_json.py <html_root> [--base-url /] +""" + +from __future__ import annotations + +import argparse +import json +from datetime import datetime +from pathlib import Path + +from packaging import version as pkg_version + +ROOT_VERSION = "main" +DEFAULT_BASE_URL = "/" +METADATA_NAME = "versions_metadata.json" + + +def _parse_creatordate(raw: str) -> datetime: + try: + return datetime.strptime(raw, "%Y-%m-%d %H:%M:%S %z") + except Exception: + return datetime.min.replace(tzinfo=None) + + +def _load_versions(metadata_path: Path) -> list[dict[str, object]]: + metadata = json.loads(metadata_path.read_text(encoding="utf-8")) + versions = [] + for name, entry in metadata.items(): + version_label = entry.get("version") or name + if version_label in {"0.0.0", "0+unknown"}: + version_label = name + versions.append( + { + "name": name, + "version": version_label, + "is_released": bool(entry.get("is_released")), + "creatordate": _parse_creatordate(entry.get("creatordate", "")), + } + ) + return versions + + +def _pick_preferred(versions: list[dict[str, object]], latest: str) -> str: + released = [v for v in versions if v["is_released"]] + if released: + return max(released, key=lambda v: v["creatordate"])["name"] + for v in versions: + if v["name"] == latest: + return latest + return versions[0]["name"] + + +def _to_switcher( + versions: list[dict[str, object]], preferred_name: str, base_url: str +) -> list[dict[str, object]]: + base = base_url.rstrip("/") + main_entry: dict[str, object] | None = None + tag_entries: list[dict[str, object]] = [] + + for v in versions: + entry = { + "name": v["name"], + "version": v["version"], + "url": f"{base}/{v['name']}/" if base else f"/{v['name']}/", + "preferred": v["name"] == preferred_name, + } + if v["name"] == "main": + main_entry = entry + else: + tag_entries.append(entry) + + def _sort_key(entry: dict[str, object]) -> pkg_version.Version: + name = str(entry["name"]) + label = name[1:] if name.startswith("v") else name + try: + return pkg_version.parse(label) + except Exception: Review Comment:  Using a broad `except Exception:` can mask unexpected errors. The `packaging.version.parse` function raises `packaging.version.InvalidVersion` for invalid version strings. It's better to catch this specific exception for more precise error handling. ```suggestion except pkg_version.InvalidVersion: ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
