chore: Filter out non-semver versions in update_generation_config.sh #4024
Conversation
Summary of ChangesHello @blakeli0, 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 addresses a critical issue in the version selection logic used for generating client libraries. By refining how the latest released version of 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 updates the script for finding the latest version of Maven artifacts to filter out pre-release versions. The change replaces xmllint with a shell pipeline using grep, sed, sort, and tail to parse versions from maven-metadata.xml.
My review suggests a significant simplification. Instead of manually parsing and filtering versions, the script can directly use the <release> tag from the Maven metadata. This tag is designed to provide the latest stable release version, which is exactly the goal here. This can be achieved with a single xmllint command, making the script more robust, maintainable, and aligned with Maven standards.
| # 1. Extract all version tags | ||
| # 2. Strip the XML tags to leave just the version numbers | ||
| # 3. Filter for strictly numbers.numbers.numbers (e.g., 2.54.0) | ||
| # 4. Sort by version (V) and take the last one (tail -n 1) | ||
| latest=$(echo "${xml_content}" \ | ||
| | grep -oE '<version>[0-9]+\.[0-9]+\.[0-9]+</version>' \ | ||
| | sed -E 's/<[^>]+>//g' \ | ||
| | sort -V \ | ||
| | tail -n 1) |
There was a problem hiding this comment.
This pipeline of grep, sed, sort, and tail to find the latest stable version is complex and potentially brittle. A much simpler and more robust approach is to leverage the <release> tag within the maven-metadata.xml file. According to the Maven metadata specification, the <release> tag contains the latest release version, which excludes pre-release versions like RCs and snapshots. This is exactly what's needed here.
You can replace this entire block with a single xmllint command, which is already a dependency for this script.
| # 1. Extract all version tags | |
| # 2. Strip the XML tags to leave just the version numbers | |
| # 3. Filter for strictly numbers.numbers.numbers (e.g., 2.54.0) | |
| # 4. Sort by version (V) and take the last one (tail -n 1) | |
| latest=$(echo "${xml_content}" \ | |
| | grep -oE '<version>[0-9]+\.[0-9]+\.[0-9]+</version>' \ | |
| | sed -E 's/<[^>]+>//g' \ | |
| | sort -V \ | |
| | tail -n 1) | |
| latest=$(xmllint --xpath 'string(metadata/versioning/release)' - <<< "${xml_content}") |
There was a problem hiding this comment.
The suggestion does not take pre-release versions like -rc into consideration.
|
|
2 participants
Filter out non-semver versions in update_generation_config.sh. This is a follow up fix for googleapis/google-cloud-java#11831. The original logic caused the generation PR googleapis/google-cloud-java#11822 on the main branch to get the RC version of gapic-generator-java and fail the compilation.