-
Notifications
You must be signed in to change notification settings - Fork 26
Implement Schema Version Update Automation #276
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| name: Auto Version Bump | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [labeled] | ||
|
|
||
| jobs: | ||
| bump-version: | ||
| if: > | ||
| github.event.label.name == 'patch' || | ||
| github.event.label.name == 'minor' || | ||
| github.event.label.name == 'major' | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Run version bump script | ||
| run: | | ||
| python tools/bump_version.py ${{ github.event.label.name }} | ||
|
|
||
| - name: Create Pull Request | ||
| uses: peter-evans/create-pull-request@v5 | ||
| with: | ||
| commit-message: "chore: bump version (${{ github.event.label.name }})" | ||
| title: "chore: bump version (${{ github.event.label.name }})" | ||
| body: | | ||
| This version bump was triggered by PR #${{ github.event.pull_request.number }} | ||
| Label applied: **${{ github.event.label.name }}** | ||
| labels: version-bump | ||
| base: main | ||
| branch: auto-version-bump/${{ github.event.pull_request.number }} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,78 @@ | ||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # The GitHub label passed as an argument: patch, minor, major | ||||||||||||||||||||||||
| LABEL = sys.argv[1] | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # The file that stores the current version | ||||||||||||||||||||||||
| VERSION_FILE = "version.txt" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def read_version(): | ||||||||||||||||||||||||
| """Read the current version from version.txt""" | ||||||||||||||||||||||||
| with open(VERSION_FILE, "r") as f: | ||||||||||||||||||||||||
| return f.read().strip() | ||||||||||||||||||||||||
|
Comment on lines
+11
to
+12
|
||||||||||||||||||||||||
| with open(VERSION_FILE, "r") as f: | |
| return f.read().strip() | |
| try: | |
| with open(VERSION_FILE, "r") as f: | |
| return f.read().strip() | |
| except FileNotFoundError: | |
| print(f"Error: {VERSION_FILE} not found. Please ensure the version file exists.", file=sys.stderr) | |
| sys.exit(1) | |
| except OSError as e: | |
| print(f"Error reading {VERSION_FILE}: {e}", file=sys.stderr) | |
| sys.exit(1) |
Copilot
AI
Dec 13, 2025
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.
Missing error handling for malformed version strings. If version.txt contains a version string that doesn't have exactly 3 parts separated by dots (e.g., "0.5" or "0.5.0.1"), the script will crash with an IndexError when trying to access parts[2]. Add validation to ensure the version string is well-formed before parsing.
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.
nit: maybe instead we could have just 3 new variables, and we should probably throw an exception for unknown LABELs
new_patch = 0
new_minor = 0
new_major = 0
if LABEL == "patch":
new_patch = patch + 1
elif LABEL == "minor":
new_minor = minor + 1
elif LABEL == "major":
new_major = major + 1
else
raise ExceptionType(f"invalid LABEL: {LABEL}")
return new_major, new_minor, new_patch
Additionally, we may need a suffix or prefix potentially, e.g. "v1.0.0-beta". But I think that can come when we need it. It looks like you started some functionality for suffix, but I dont think it's done.
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.
Thanks you for the feedback!
I understand the suggestion to use three new variables, it does make the bump logic more explicit and clear. I also see your point about throwing an exception for unknown labels. In the current setup, the GitHub workflow only passes predefined labels (patch, minor, major, release), so in practice an exception isn’t strictly necessary.
That said, adding an exception could serve as a useful safeguard if someone were to run the script manually with an invalid label. I’ll update the script to incorporate both of these suggestions.
Copilot
AI
Dec 13, 2025
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.
This assignment to 'new_patch' is unnecessary as it is redefined before this value is used.
This assignment to 'new_patch' is unnecessary as it is redefined before this value is used.
This assignment to 'new_patch' is unnecessary as it is redefined before this value is used.
| new_patch = patch |
Copilot
AI
Dec 13, 2025
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.
Missing implementation for 'release' label handling. The PR description states "For release labels, the workflow removes the -draft suffix from the version," but there is no code to handle the 'release' label. The script only processes 'patch', 'minor', and 'major' labels, and will raise a ValueError if 'release' is passed.
Copilot
AI
Dec 13, 2025
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 version suffix is being lost during the bump process. The PR description states "Non-release bumps retain any existing suffix (e.g., -draft) while incrementing the appropriate version component," but the current implementation removes the suffix in parse_version (line 25) and never restores it when constructing the new version (line 68). The suffix should be preserved and appended back to the new version string.
Copilot
AI
Dec 13, 2025
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.
Misleading comment about race condition prevention. The comment claims this check "prevents race conditions," but comparing version strings doesn't prevent race conditions in concurrent workflows. The check only avoids unnecessary writes when the version hasn't changed (which shouldn't happen in normal operation). Consider updating the comment to accurately describe what this check does.
| # Prevent race conditions by only writing if version changed | |
| # Only write if the version has changed to avoid unnecessary writes |
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.
Inconsistent checkout action version. This workflow uses actions/checkout@v4, but the existing workflow in .github/workflows/main.yml uses actions/checkout@v5. For consistency and to ensure the latest features and security fixes, consider using v5 here as well.