chore: address product documentation in googleapis-common-protos#16841
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes product documentation metadata and associated links for the googleapis-common-protos package. While the changes successfully remove the intended documentation lines, they leave behind empty hyperlink targets and references in both the root and documentation README.rst files, which results in invalid reStructuredText syntax. Feedback recommends expanding the post-processing configuration to cover both README files and to remove these broken references and targets to ensure a clean build.
jskeet
force-pushed
the
remove-googleapis-common-protos-documentation
branch
from
9fa8cea to
f379582
Compare
jskeet
force-pushed
the
remove-googleapis-common-protos-documentation
branch
3 times, most recently
from
8597f6c to
087091a
Compare
|
Will fix the merge conflict |
| - paths: [ | ||
| packages/googleapis-common-protos/README.rst | ||
| ] | ||
| before: ".. _Google APIs Common Protos Product documentation: \n" | ||
| after: "" | ||
| count: 1 |
There was a problem hiding this comment.
I’m concerned about introducing additional post-processing steps at this stage of the librarian migration. Since the common protos README was previously maintained by hand, could we revert to the manual version for now? I’d prefer to wait until we have a robust way to support autogeneration for proto-only libraries rather than adding more customizations.
There was a problem hiding this comment.
I'd be happy to revert to a manual version, yes - there's a lot about this file which isn't great. But how do we stop the post-processor from generating the readme?
There was a problem hiding this comment.
My mistake, the README is autogenerated from synthtool here: https://github.com/googleapis/synthtool/blob/ba9afbd284495433734b84686ef8ef6785c60128/synthtool/languages/python_mono_repo.py#L231-L254
To switch to a handwritten README, we would need to modify synthtool to skip this logic for proto-only libraries. Since this is an existing issue, perhaps we don't need all of the post processing here unless we want to fix it properly. In that case, the fix should be in synthtool where we skip the code below for proto-only packages
There was a problem hiding this comment.
To switch to a handwritten README, we would need to modify synthtool to skip this logic for proto-only libraries. Since this is an existing issue, perhaps we don't need all of the post processing here unless we want to fix it properly. In that case, the fix should be in synthtool where we skip the code below for proto-only packages
I'd rather fix it "hackily" for now (with this PR), and then do it properly when we rewrite bits of synthtool in Go (and ideally remove all kinds of string replacements - including this one). Are you okay with that?
There was a problem hiding this comment.
Ack. I see the follow up work is captured in googleapis/librarian#5670 (comment). I'll approve this so that it doesn't block the overall migration effort.
- Remove the product_documentation_override - Add a string replacement to remove the missing link line Completes the google-cloud-python part of googleapis#5466 Filed googleapis/librarian#5670 to remove the need for the string replacement when we migrate synthtool code into librarian.
jskeet
force-pushed
the
remove-googleapis-common-protos-documentation
branch
from
087091a to
e7cb3bb
Compare
2 participants
Completes the google-cloud-python part of #5466
Filed googleapis/librarian#5670 to remove the need for the string replacement when we migrate synthtool code into librarian.