-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix version bump script to handle TOML files with outdated versions #11787
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
base: main
Are you sure you want to change the base?
Conversation
The script previously failed to update pyproject_xpu.toml and pyproject_cpu.toml because it used simple string replacement that only worked when files contained the exact current version. Changes: - Update replace_in_file() to use regex for TOML files, matching the version field pattern instead of exact version string - Add validation at end of bump_version() to verify all files were updated correctly, failing if any file doesn't contain new version - Keep backward compatibility for non-TOML files This ensures all platform-specific pyproject files stay synchronized regardless of their current version state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary of ChangesHello @Kangyan-Zhou, 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 reliability of the version bumping script by introducing more robust handling for TOML configuration files. It specifically addresses an issue where the script failed to update 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request enhances the version bumping script to correctly handle TOML files and adds validation to ensure all specified files are updated. The changes include using regex for TOML files to match the version field and adding a validation step to verify that all files contain the new version after the update. This ensures that platform-specific pyproject files stay synchronized regardless of their current version state.
scripts/release/utils.py
Outdated
continue # Skip non-existent files (already warned above) | ||
|
||
content = file_abs.read_text() | ||
if new_version not in content: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation check if new_version not in content:
is too simple. It's possible that the new_version
string appears elsewhere in the file. It would be more robust to use a regex to specifically check the version field in the TOML file, similar to how it's done in replace_in_file
function.
if file_abs.suffix == ".toml":
pattern = r'(version\s*=\s*)(["']?)([^"'\n]+)(["']?)'
if not re.search(pattern, content.replace(new_version, rf"\g<1>{new_version}\g<2>")):
failed_files.append(file_rel)
print(f"✗ {file_rel} does not contain version {new_version}")
else:
if new_version not in content:
failed_files.append(file_rel)
print(f"✗ {file_rel} does not contain version {new_version}")
scripts/release/utils.py
Outdated
# For TOML files, use regex to match version field regardless of current value | ||
if file_path.suffix == ".toml": | ||
# Match: version = "X.Y.Z..." (with optional quotes and whitespace) | ||
pattern = r'(version\s*=\s*["\'])([^"\']+)(["\'])' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern r'(version\s*=\s*[""])([^""]+)([""])'
could be improved to be more robust. Currently, it only matches double or single quotes but might fail if the TOML file has a different quote style or no quotes at all. Consider allowing no quotes or other quote types (e.g., single quotes).
pattern = r'(version\s*=\s*)(["']?)([^"'\n]+)(["']?)'
scripts/release/utils.py
Outdated
if file_path.suffix == ".toml": | ||
# Match: version = "X.Y.Z..." (with optional quotes and whitespace) | ||
pattern = r'(version\s*=\s*["\'])([^"\']+)(["\'])' | ||
new_content = re.sub(pattern, rf"\g<1>{new_version}\g<3>", content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The substitution rf"\g<1>{new_version}\g<3>"
assumes that the quotes are always present in the original string. If the TOML file does not have quotes around the version, this substitution will add them. It would be better to preserve the original quoting style (or lack thereof).
new_content = re.sub(pattern, rf"\g<1>{new_version}\g<2>", content)
failed_files = [] | ||
for file_rel in files_to_update: | ||
file_abs = repo_root / file_rel | ||
if not file_abs.exists(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The continue
statement skips non-existent files, but this might hide potential issues. It would be better to log a warning or error message to inform the user that a file specified for update does not exist. This can help in debugging configuration issues.
if not file_abs.exists():
print(f"Warning: File {file_rel} does not exist, skipping validation.")
continue # Skip non-existent files (already warned above)
The script previously failed to update pyproject_xpu.toml and pyproject_cpu.toml because it used simple string replacement that only worked when files contained the exact current version.
This ensures all platform-specific pyproject files stay synchronized regardless of their current version state.
Tested in #11802