atridad/Discodrome
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

Automate manifest.json version updates during release #9

Merged
deluan merged 6 commits from claude/automate-manifest-versioning-MJ9WM into main 2026-02-07 18:17:09 -07:00
Conversation 4 Commits 6 Files Changed 3
+14 -1
2 changed files with 14 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 9ee12f2a31 - Show all commits
.github/workflows/release.yml
+9
View File
@@ -16,6 +16,15 @@ jobs:
- name: Check out code
copilot-pull-request-reviewer[bot] commented 2026-02-07 15:53:33 -07:00 (Migrated from github.com)
Review
Copy Link
Copy Source

MANIFEST_VERSION is extracted via grep -oP with a PCRE-specific \K, which is brittle for JSON (and can fail if GNU grep isn’t built with PCRE support or if formatting changes). Consider parsing manifest.json with a JSON-aware tool (e.g., jq -r .version manifest.json or a short Python snippet) to make this guard reliable.

                  MANIFEST_VERSION=$(jq -r '.version' manifest.json)
`MANIFEST_VERSION` is extracted via `grep -oP` with a PCRE-specific `\K`, which is brittle for JSON (and can fail if GNU grep isn’t built with PCRE support or if formatting changes). Consider parsing `manifest.json` with a JSON-aware tool (e.g., `jq -r .version manifest.json` or a short Python snippet) to make this guard reliable. ```suggestion MANIFEST_VERSION=$(jq -r '.version' manifest.json) ```
uses: actions/checkout@v5
- name: Verify manifest version matches tag
run: |
TAG_VERSION="${GITHUB_REF_NAME#v}"
MANIFEST_VERSION=$(grep -oP '"version"\s*:\s*"\K[^"]+' manifest.json)
if [ "$TAG_VERSION" != "$MANIFEST_VERSION" ]; then
echo "::error::Tag version ($TAG_VERSION) does not match manifest.json version ($MANIFEST_VERSION)"
exit 1
fi
- name: Set up Go
uses: actions/setup-go@v5
with:
Makefile
+5 -1
View File
@@ -19,6 +19,10 @@ release: test
@if [[ ! "${V}" =~ ^[0-9]+\.[0-9]+\.[0-9]+.*$$ ]]; then echo "Usage: make release V=X.X.X"; exit 1; fi
gemini-code-assist[bot] commented 2026-02-07 15:50:48 -07:00 (Migrated from github.com)
Review
Copy Link
Copy Source

critical

The shell variable ${V} will not be expanded within single quotes. This will result in the literal string "${V}" being written to manifest.json instead of the version number. To fix this, you should use double quotes for the sed expression and escape the inner double quotes.

Additionally, sed -i behaves differently on GNU/Linux versus macOS/BSD systems. For better portability, you might consider a different approach in the future, but fixing the variable expansion is the critical part.

@sed -i "s/\"version\": *\"[^\"]*\"/\"version\": \"${V}\"/" manifest.json
![critical](https://www.gstatic.com/codereviewagent/critical.svg) The shell variable `${V}` will not be expanded within single quotes. This will result in the literal string `"${V}"` being written to `manifest.json` instead of the version number. To fix this, you should use double quotes for the `sed` expression and escape the inner double quotes. Additionally, `sed -i` behaves differently on GNU/Linux versus macOS/BSD systems. For better portability, you might consider a different approach in the future, but fixing the variable expansion is the critical part. ``` @sed -i "s/\"version\": *\"[^\"]*\"/\"version\": \"${V}\"/" manifest.json ```
copilot-pull-request-reviewer[bot] commented 2026-02-07 15:53:32 -07:00 (Migrated from github.com)
Review
Copy Link
Copy Source

git push origin main v${V} pushes the local main ref, but the release commit is created on the current checked-out branch. If the user runs make release from a branch other than main, the tag will point to the release commit but origin/main will not receive that commit (and could even push an outdated local main). Consider either enforcing/releases from main (fail if not on main and git pull --ff-only), or push HEAD:main alongside the tag so the commit you just made is what gets pushed.

`git push origin main v${V}` pushes the local `main` ref, but the release commit is created on the *current* checked-out branch. If the user runs `make release` from a branch other than `main`, the tag will point to the release commit but `origin/main` will not receive that commit (and could even push an outdated local `main`). Consider either enforcing/releases from `main` (fail if not on `main` and `git pull --ff-only`), or push `HEAD:main` alongside the tag so the commit you just made is what gets pushed.
copilot-pull-request-reviewer[bot] commented 2026-02-07 15:53:33 -07:00 (Migrated from github.com)
Review
Copy Link
Copy Source

The release recipe uses bash-only syntax ([[ ... ]] / =~) and GNU sed -i semantics. Since the Makefile doesn’t set SHELL := bash, this target can fail on systems where /bin/sh is dash (common on Ubuntu) or where sed -i requires a backup suffix (macOS/BSD). Consider setting SHELL := /usr/bin/env bash (or rewriting to POSIX sh) and using a portable in-place edit approach for manifest.json.

The `release` recipe uses bash-only syntax (`[[ ... ]]` / `=~`) and GNU `sed -i` semantics. Since the Makefile doesn’t set `SHELL := bash`, this target can fail on systems where `/bin/sh` is `dash` (common on Ubuntu) or where `sed -i` requires a backup suffix (macOS/BSD). Consider setting `SHELL := /usr/bin/env bash` (or rewriting to POSIX sh) and using a portable in-place edit approach for `manifest.json`.
go mod tidy
@if [ -n "`git status -s`" ]; then echo "\n\nThere are pending changes. Please commit or stash first"; exit 1; fi
@# Update version in manifest.json
@sed -i 's/"version": *"[^"]*"/"version": "${V}"/' manifest.json
git add manifest.json
git commit -m "Release v${V}" --allow-empty --no-verify
git tag v${V}
git push origin v${V} --no-verify
git push origin main v${V} --no-verify
.PHONY: release