[do not merge]: Dry run for packages/gcp-sphinx-docfx-yaml migration#16059
[do not merge]: Dry run for packages/gcp-sphinx-docfx-yaml migration#16059
Conversation
* fix: complete toc disambiguation * fix: update to increment values * chore: update names map to name_entries * test: update to use unittest * test: fix typo in ci.yaml * test: update name and test file lengths
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* fix: update dependency requirements * test: add test for using without requirements.txt * test: replace original set up for this PR
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* test: update Dockerfile * ci: update Dockerfile * chore: update formatting
* fix: correct parser to scan specific tokens only * fix: update parser for varying input types * test: add unittest for extract_docstring_info * fix: update parser and test * fix: update to return summary directly
* fix: add addtional handler for GoogleDocstring * test: add unittest * fix: simplify function call and references in test * fix: update test to include xref version * fix: update variable name to lowercase
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
#65) Before you open a pull request, note that this repository is forked from [here](https://github.com/docascode/sphinx-docfx-yaml/). Unless the issue you're trying to solve is unique to this specific repository, please file an issue and/or send changes upstream to the original as well. __________________________________________________________________ Python-api-core is another repo that is not using templated Noxfile.py for various reasons (see discussion on googleapis/python-api-core#219), and we'll need to update it manually when we need Sphinx version updates. > It's a good idea to open an issue first for discussion. - [x] Tests pass - [ ] Appropriate changes to README are included in PR
* chore: allow testing plugin without running nox * chore: update comment * chore: Update .kokoro/generate-docs.sh Co-authored-by: Tyler Bui-Palsulich <26876514+tbpg@users.noreply.github.com> Co-authored-by: Tyler Bui-Palsulich <26876514+tbpg@users.noreply.github.com>
* feat: add short snippet for missing summary * feat: add summary for index pages
* feat: deduplicate all entry names in yaml_data * test: update unittest for disambiguate_toc_name
Co-authored-by: Dan Lee <71398022+dandhlee@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Dan Lee <71398022+dandhlee@users.noreply.github.com>
* feat: add subPackage types * fix: update type in summary and use markdown to highlight names * fix: use short and disambiguated names * nit: update to f string for single-line string
* feat: add subclasses to children and reference * chore: lint * nit: update comment to clarify confusion
Co-authored-by: Dan Lee <71398022+dandhlee@users.noreply.github.com>
* accept change from upstream * feat: add full support for xrefs * test: update test case * fix: expand xref format for proper processing * test: update unit test * fix: handle xrefs for other products in the future
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* fix: allow bracketed xref to work * test: update unit test * test: update to include xref checking in unit test
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* fix: parse xrefs differently with new xref format * fix: update Docstring parser * test: update unit test * fix: move error catching to another block * test: update unittest to include error scenarios * fix: clean up messy formatting
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@71a7297 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:e8dcfd7cbfd8beac3a3ff8d3f3185287ea0625d859168cc80faccfc9a7a00455 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Bumps [django](https://github.com/django/django) from 3.2.25 to 4.2.16. - [Commits](django/django@3.2.25...4.2.16) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
#403) Source-Link: googleapis/synthtool@de3def6 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:a1c5112b81d645f5bbc4d4bbc99d7dcb5089a52216c0e3fb1203a0eeabadd7d5 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Bumps [virtualenv](https://github.com/pypa/virtualenv) from 20.17.1 to 20.26.6. - [Release notes](https://github.com/pypa/virtualenv/releases) - [Changelog](https://github.com/pypa/virtualenv/blob/main/docs/changelog.rst) - [Commits](pypa/virtualenv@20.17.1...20.26.6) --- updated-dependencies: - dependency-name: virtualenv dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [django](https://github.com/django/django) from 4.2.16 to 4.2.18. - [Commits](django/django@4.2.16...4.2.18) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.4 to 3.1.5. - [Release notes](https://github.com/pallets/jinja/releases) - [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst) - [Commits](pallets/jinja@3.1.4...3.1.5) --- updated-dependencies: - dependency-name: jinja2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@aa69fb7 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f016446d6e520e5fb552c45b110cba3f217bffdd3d06bdddd076e9e6d13266cf Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Bumps [h11](https://github.com/python-hyper/h11) from 0.14.0 to 0.16.0. - [Commits](python-hyper/h11@v0.14.0...v0.16.0) --- updated-dependencies: - dependency-name: h11 dependency-version: 0.16.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix: add support for formatting broken argspec entities * test: add unit test * test: update unit test
Bumps [django](https://github.com/django/django) from 4.2.18 to 4.2.25. - [Commits](django/django@4.2.18...4.2.25) --- updated-dependencies: - dependency-name: django dependency-version: 4.2.25 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Daniel Sanche <d.sanche14@gmail.com> Co-authored-by: Daniel Sanche <sanche@google.com>
* chore: cleanup string.format to f-strings * chore: fix cases where fstrings are not possible
* chore: add type hints and docstrings * chore: fix minor adjustments * test: revert unintended changes
Source-Link: googleapis/synthtool@6702a34 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:fbbc8db67afd8b7d71bf694c5081a32da0c528eba166fbcffb3b6e56ddf907d5 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Lingqing Gan <lingqing.gan@gmail.com>
Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.5 to 3.1.6. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/pallets/jinja/releases">jinja2's releases</a>.</em></p> <blockquote> <h2>3.1.6</h2> <p>This is the Jinja 3.1.6 security release, which fixes security issues but does not otherwise change behavior and should not result in breaking changes compared to the latest feature release.</p> <p>PyPI: <a href="https://pypi.org/project/Jinja2/3.1.6/">https://pypi.org/project/Jinja2/3.1.6/</a> Changes: <a href="https://jinja.palletsprojects.com/en/stable/changes/#version-3-1-6">https://jinja.palletsprojects.com/en/stable/changes/#version-3-1-6</a></p> <ul> <li>The <code>|attr</code> filter does not bypass the environment's attribute lookup, allowing the sandbox to apply its checks. <a href="https://github.com/pallets/jinja/security/advisories/GHSA-cpwx-vrp4-4pq7">https://github.com/pallets/jinja/security/advisories/GHSA-cpwx-vrp4-4pq7</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/pallets/jinja/blob/main/CHANGES.rst">jinja2's changelog</a>.</em></p> <blockquote> <h2>Version 3.1.6</h2> <p>Released 2025-03-05</p> <ul> <li>The <code>|attr</code> filter does not bypass the environment's attribute lookup, allowing the sandbox to apply its checks. :ghsa:<code>cpwx-vrp4-4pq7</code></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/pallets/jinja/commit/15206881c006c79667fe5154fe80c01c65410679"><code>1520688</code></a> release version 3.1.6</li> <li><a href="https://github.com/pallets/jinja/commit/90457bbf33b8662926ae65cdde4c4c32e756e403"><code>90457bb</code></a> Merge commit from fork</li> <li><a href="https://github.com/pallets/jinja/commit/065334d1ee5b7210e1a0a93c37238c86858f2af7"><code>065334d</code></a> attr filter uses env.getattr</li> <li><a href="https://github.com/pallets/jinja/commit/033c20015c7ca899ab52eb921bb0f08e6d3dd145"><code>033c200</code></a> start version 3.1.6</li> <li><a href="https://github.com/pallets/jinja/commit/bc68d4efa99c5f77334f0e519628558059ae8c35"><code>bc68d4e</code></a> use global contributing guide (<a href="https://redirect.github.com/pallets/jinja/issues/2070">#2070</a>)</li> <li><a href="https://github.com/pallets/jinja/commit/247de5e0c5062a792eb378e50e13e692885ee486"><code>247de5e</code></a> use global contributing guide</li> <li><a href="https://github.com/pallets/jinja/commit/ab8218c7a1b66b62e0ad6b941bd514e3a64a358f"><code>ab8218c</code></a> use project advisory link instead of global</li> <li><a href="https://github.com/pallets/jinja/commit/b4ffc8ff299dfd360064bea4cd2f862364601ad2"><code>b4ffc8f</code></a> release version 3.1.5 (<a href="https://redirect.github.com/pallets/jinja/issues/2066">#2066</a>)</li> <li>See full diff in <a href="https://github.com/pallets/jinja/compare/3.1.5...3.1.6">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) You can trigger a rebase of this PR by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/googleapis/sphinx-docfx-yaml/network/alerts). </details> > **Note** > Automatic rebases have been disabled on this pull request as it has been open for over 30 days. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Victor Chudnovsky <vchudnov@google.com>
Bug ID: b/479543683
Replace old teams (dkp, onramp) with dkp-team. b/478003109
…n' into migration.sphinx-docfx-yaml.migration.2026-03-09_20-55-24.migrate
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on migrating the sphinx-docfx-yaml code from its original repository to a new package, Highlights
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request appears to be a migration of the gcp-sphinx-docfx-yaml package. The changes consist of adding numerous configuration files for CI/CD, repository settings, and the full source code for the Python package. My review focuses on improving code correctness, security, and adherence to best practices. I've identified a few issues, including a potential security vulnerability in a script due to the use of an unsafe YAML loading function, incorrect logic for handling duplicate entries, and some packaging and scripting improvements.
Note: Security Review did not run due to the size of the PR.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/gcp-sphinx-docfx-yaml/.kokoro/trampoline.sh (27)
The use of command substitution $(...) here is incorrect. The populate-secrets.sh script performs actions (writes secrets to files) and prints informational messages to stderr, but it does not produce any command on stdout to be executed. The current code works by chance because the stdout is empty. To execute the script for its side effects, you should call it directly. I've also added quotes around dirname $0 for robustness against paths with spaces.
"$(dirname "$0")"/populate-secrets.sh # Secret Manager secrets.
packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py (1061-1063)
The code checks for and skips the cls parameter for class methods, but it doesn't do the same for the self parameter for instance methods. This should be handled consistently to avoid processing the self parameter as a regular argument. While self is handled later in build_finished, it's better to handle it here for consistency.
if arg in ('self', 'cls') or arg not in type_map:
default_index += 1
continue
packages/gcp-sphinx-docfx-yaml/docfx_yaml/monkeypatch.py (287-290)
The logic to prevent adding duplicate references is incorrect. The condition any(r['uid'] != _added_reference['uid'] for r in data['references']) will be true as long as there is at least one other different reference in the list, which can lead to the same reference being added multiple times.
The correct logic should check if the new reference's UID is not already present in the list.
This same logic error is repeated on lines 311-314 and 327-330.
if not any(r['uid'] == _added_reference['uid'] for r in data['references']):
data['references'].append(_added_reference)
packages/gcp-sphinx-docfx-yaml/scripts/readme-gen/readme_gen.py (57)
Using yaml.load() is unsafe as it can execute arbitrary code if the YAML file is malicious. It's strongly recommended to use yaml.safe_load() instead to prevent potential security vulnerabilities.
config = yaml.safe_load(f)
packages/gcp-sphinx-docfx-yaml/.kokoro/generate-docs.sh (91)
For command substitution, it is generally recommended to use $(...) instead of backticks (`...`). The $(...) syntax is more readable, can be nested, and avoids issues with backslashes.
GITHUB_TAGS=$(git describe --tags $(git rev-list --tags --max-count=1))
packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py (432)
There is a typo in the error message. "enoucntered" should be "encountered".
raise ValueError(f"Wrong formatting encountered for \n{line}")
packages/gcp-sphinx-docfx-yaml/docfx_yaml/writer.py (1157)
Instead of using print for system messages, it's better to use Sphinx's built-in logging mechanism, such as self.builder.warn(). This integrates with Sphinx's logging, allows for better control over verbosity, and can turn warnings into errors with the -W flag.
self.builder.warn("System message warnings: %s" % node.astext())
packages/gcp-sphinx-docfx-yaml/setup.py (22)
black is a code formatter and is typically considered a development dependency, not a runtime dependency for a library. Including it in install_requires adds an unnecessary dependency for users of your package. It should be moved to a development requirements file (e.g., requirements-dev.txt) or specified in extras_require for a dev extra.
packages/gcp-sphinx-docfx-yaml/setup.py (35)
wheel is a build-time dependency used for creating wheel distributions. It should not be listed as a runtime dependency in install_requires. According to modern Python packaging standards (PEP 517/518), it should be specified in the [build-system] table of a pyproject.toml file.
See #15998.
This PR should be merged with a merge-commit, not a squash-commit, in order to preserve the git history.