Repository navigation
fix(ci): add VERSION_OVERRIDE to check-versions - #764
Conversation
…ntrol When commits use `feat:` conventional prefix but the changes are minor (e.g., small additions to existing features), the auto-detected bump level can be too aggressive (minor instead of patch). This adds a VERSION_OVERRIDE parameter that bypasses conventional commit detection and uses the specified version for all bumps. What changed: - Makefile: check-versions target accepts VERSION_OVERRIDE parameter, passes it as an environment variable to the script - scripts/check_release_versions.sh: bump_version() returns the override when VERSION_OVERRIDE is set, skipping level-based computation. Displays override notice in output header. Usage: make check-versions VERSION_OVERRIDE=1.3.1 Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
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 significantly enhances the version checking process within the CI pipeline by introducing a Highlights
Changelog
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
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMakefile's check-versions target now accepts an optional VERSION_OVERRIDE environment variable and always invokes Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 028cfb0dcd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request introduces a VERSION_OVERRIDE feature to the check-versions script, allowing manual control over version bumps. The implementation is mostly correct, but I've identified a couple of areas for improvement. In the Makefile, the script invocation can be significantly simplified for better readability and maintainability. More critically, the check_release_versions.sh script is missing validation for the VERSION_OVERRIDE input, which could lead to corrupted version files if an invalid format is provided. This aligns with our rule on ensuring accurate external resource versions to prevent build failures. My review includes suggestions to address both of these points.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check_release_versions.sh`:
- Around line 229-237: The bump_version function currently returns
VERSION_OVERRIDE without validation; modify bump_version to validate
VERSION_OVERRIDE against a semantic version regex before echoing it. In the
bump_version function, when VERSION_OVERRIDE is non-empty, check it matches a
semver pattern (e.g., MAJOR.MINOR.PATCH with optional pre-release/build if your
project allows) and if valid echo it, otherwise fail early (non-zero exit or
error message via echo/stderr) so invalid values are rejected; reference the
bump_version function and the VERSION_OVERRIDE variable when implementing this
check.
- Around line 126-128: The script reads VERSION_OVERRIDE (in the if [[ -n
"$VERSION_OVERRIDE" ]] block) before it's defined, which breaks under set -u;
move the initialization VERSION_OVERRIDE="${VERSION_OVERRIDE:-}" so it runs
before that if check (i.e., initialize VERSION_OVERRIDE early in the script or
at least before the conditional that echoes the override) to ensure the variable
is always defined when referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: de051a04-7532-4387-97d6-b95d2d969f93
📒 Files selected for processing (2)
Makefilescripts/check_release_versions.sh
What changed: - scripts/check_release_versions.sh: move VERSION_OVERRIDE initialization before first use (fixes unbound variable error under set -u), add X.Y.Z format validation to reject invalid version strings - Makefile: simplify check-versions target to single line by passing VERSION_OVERRIDE as env var and TAG as positional arg directly Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bee4fd6d6f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ./scripts/check_release_versions.sh; \ | ||
| fi | ||
| check-versions: ## Check workspace crate versions against latest tag (usage: make check-versions [TAG=v1.0.0] [VERSION_OVERRIDE=1.3.1]) | ||
| @VERSION_OVERRIDE="$(VERSION_OVERRIDE)" ./scripts/check_release_versions.sh $(TAG) |
There was a problem hiding this comment.
Quote TAG when invoking version-check script
The new check-versions recipe passes $(TAG) to the shell unquoted, so any shell metacharacters in the value are interpreted by /bin/sh instead of being treated as a single tag argument. This is a regression from the previous implementation, which quoted "$(TAG)"; for example, make check-versions TAG='release; echo injected' runs an extra command and does not validate the intended tag.
Useful? React with 👍 / 👎.
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Summary
VERSION_OVERRIDEparameter tomake check-versionsthat bypasses conventional commit detection and uses the specified version for all bumpsfeat:commits trigger a minor bump (e.g., 1.3.0 → 1.4.0) when only a patch bump is warranted (1.3.0 → 1.3.1)What changed
check-versionstarget acceptsVERSION_OVERRIDE, exports it as env var to the scriptbump_version()returns the override whenVERSION_OVERRIDEis set, skipping level-based computationUsage
Test plan
make check-versionswithout override behaves as before (auto-detect from commits)make check-versions VERSION_OVERRIDE=1.3.1proposes 1.3.1 for all unbumped cratesSummary by CodeRabbit