Skip to content
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

feat: grab repository metadata #364

Merged
merged 3 commits into from
Apr 1, 2024
Merged

feat: grab repository metadata #364

merged 3 commits into from
Apr 1, 2024

Conversation

dandhlee
Copy link
Collaborator

@dandhlee dandhlee commented Mar 30, 2024

I've noticed an issue with the rest of the libraries not being able to properly find the library names from the given Sphinx info.

Seems like I need to extract the .repo-metadata.json file directly to source this information - Sphinx has very limited ways to deliver this information through docs/conf.py.

Based on the googleapis/ repo search, it seems that using name suffices for shortname for the library. If I encounter ones that don't work, I'll create an exception around those few. I've sampled ~100 and seems to work with using name instead of api_shortname.

Verified that existing library generation demo for bigframes and logging still works.

If the repository metadata could not be retrieved, skip over getting the summary page details.

Fixes b/331977022.

  • Tests pass
  • Appropriate changes to README are included in PR

@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Mar 30, 2024
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Mar 30, 2024
@dandhlee dandhlee marked this pull request as ready for review March 30, 2024 02:05
@dandhlee dandhlee requested review from a team as code owners March 30, 2024 02:05
@@ -2086,9 +2106,11 @@ def convert_module_to_package_if_needed(obj):
file_name_set.add(filename)

for entry in yaml_data:
if not app.env.library_shortname:
continue

Choose a reason for hiding this comment

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

This would apply to the whole library, right? If so, I think you can just break instead of continue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, done.

@@ -26,6 +26,7 @@
import shutil
import black
import logging
import json

Choose a reason for hiding this comment

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

Nit: import ordering

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I was able to sort through the first party libraries.

Copy link
Collaborator Author

@dandhlee dandhlee left a comment

Choose a reason for hiding this comment

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

Thank you! Verified that new changes did not change the result.

@@ -26,6 +26,7 @@
import shutil
import black
import logging
import json
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I was able to sort through the first party libraries.

@@ -2086,9 +2106,11 @@ def convert_module_to_package_if_needed(obj):
file_name_set.add(filename)

for entry in yaml_data:
if not app.env.library_shortname:
continue
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, done.

@dandhlee dandhlee merged commit 1318b24 into main Apr 1, 2024
6 checks passed
@dandhlee dandhlee deleted the extract_shortname branch April 1, 2024 04:11
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.

2 participants