docs: Adds links related to logging samples in README.rst.j2#2570
docs: Adds links related to logging samples in README.rst.j2#2570chalmerlowe wants to merge 1 commit intomainfrom
Conversation
Provides three links to logging documentation and samples in `README.rst.j2` * Docs * Generated samples * Legacy OR handwritten samples
product-auto-label
bot
added
the
samples
Summary of ChangesHello @chalmerlowe, 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 enhances the Highlights
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
|
![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 adds links related to logging samples to a generic README template. While the intention is good, the added links are specific to the google-cloud-logging library. Since this template is used for all client libraries, this change will cause logging-specific links to appear in the documentation for unrelated libraries (like Translate, Vision, etc.), which could be confusing. My review comment suggests making this new section conditional, so it only appears when generating the documentation for the logging library, aligning with the rule of fixing generator templates for consistency.
| Additional information | ||
| ~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| For additional information and examples about logging see: | ||
|
|
||
| - `Logging Docs`_ | ||
| - `Generated samples`_ | ||
| - `Additional samples`_ | ||
|
|
||
| .. _Logging Docs: https://docs.cloud.google.com/logging/docs | ||
| .. _Generated samples: https://github.com/googleapis/google-cloud-python/tree/main/packages/google-cloud-logging/samples/generated_samples | ||
| .. _Additional samples: https://github.com/GoogleCloudPlatform/python-docs-samples/tree/main/logging |
There was a problem hiding this comment.
This template file, README.rst.j2, is a generic template used to generate README.rst files for all Python client libraries. The links you've added are specific to google-cloud-logging.
By adding them to this shared template, these logging-specific links will incorrectly appear in the READMEs of all other client libraries (e.g., Translate, Vision, etc.), which will be confusing for users of those libraries.
If these links are only relevant for the logging library, they should be added conditionally. For example, using a Jinja if block (the exact variable may differ):
{% if 'logging' in api.name %}
Additional information
~~~~~~~~~~~~~~~~~~~~~~
For additional information and examples about logging see:
- `Logging Docs`_
- `Generated samples`_
- `Additional samples`_
.. _Logging Docs: https://docs.cloud.google.com/logging/docs
.. _Generated samples: https://github.com/googleapis/google-cloud-python/tree/main/packages/google-cloud-logging/samples/generated_samples
.. _Additional samples: https://github.com/GoogleCloudPlatform/python-docs-samples/tree/main/logging
{% endif %}If a conditional approach isn't feasible, these links should be removed from this common template to avoid generating incorrect documentation for other libraries.
References
- For generated code, apply fixes to the generator templates rather than patching individual output files to ensure consistency across the codebase.
There was a problem hiding this comment.
@gemini-code-assist if we look at the context around these three links, we will see that this entire section of the readme is devoted to how to use logging with any client library (i.e. Translate, Vision, etc). This PR adds links for the customer to learn more about how they can apply logging, so this addition is both universal and highly relevant to this section.
There was a problem hiding this comment.
You're right that the preceding section discusses general logging configurations applicable to any client library. However, the specific links you've added for "Generated samples" and "Additional samples" point directly to the google-cloud-logging Python client library's samples.
Since this README.rst.j2 is a generic template, these logging-specific sample links will appear in the READMEs of all generated client libraries (e.g., Translate, Vision, etc.). This would be misleading and confusing for users of those other libraries, as the links would not be relevant to their specific client library.
To maintain the universal applicability of this section, I recommend one of two approaches:
- Make the sample links conditional: As suggested previously, wrap the sample links in a Jinja
ifblock that only renders them for thegoogle-cloud-logginglibrary. - Remove the logging-specific sample links: Keep only the general "Logging Docs" link, which is universally relevant. This would look like this:
Additional information
~~~~~~~~~~~~~~~~~~~~~~
For additional information and examples about logging see:
- `Logging Docs`_
.. _Logging Docs: https://docs.cloud.google.com/logging/docsThis ensures that the generated READMEs for all client libraries provide accurate and relevant information.
|
It looks like the goldens need to be updated. Do you have a workflow for that set up? (If not, I can share a script) |
|
The feedback in #2570 (comment) is valid. We should consider linking to the samples directory in the client library package where we have autogenerated snippets and https://github.com/GoogleCloudPlatform/python-docs-samples where we have handwritten code samples |
3 participants
Provides three links to logging documentation and samples in
README.rst.j2