Minor tweaks on #208 - #210
Conversation
The body is written once and omitBodyDuringUpdate keeps it that way, so a `main` link let an old release's instructions follow whatever `main` said later, which may describe a different install layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matching any Studio zip let the body and the attached asset disagree on the version, pointing testers at a download that is not there. The README's own placeholder said `<version>` where the body says `<Studio version>`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The step-level comment already covers omitBodyDuringUpdate, and the pipefail note guarded a SIGPIPE that a two-asset release cannot reach. What is left is the tag pinning, the absolute bodyFile path, and the `|| true`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughRelease publication now pins installation links to the release tag. Release verification checks unresolved placeholders and the exact attached Studio zip filename. Release instructions document these requirements. ChangesRelease validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The publish workflow substitutes the tag in, but a body copied from release-body.md by hand can publish with a `/blob/v<version>/` link to a tag that does not exist, and nothing caught it. Also swap `head -n 1` for `sed -n 1p`, which cannot SIGPIPE the grep feeding it, and cut the two Studio-zip messages down to the facts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Verify Release check for `/blob/v<version>/` landed without updating the two places the README lists what a maintainer has to fill in, so the manual path said only "version placeholders". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/verify-release.yml:
- Around line 45-48: Update the release-body validation around the existing
placeholder check in the workflow to extract the `INSTALL.md` blob URL’s tag and
compare it with `github.event.release.tag_name`. Fail the validation when the
link uses any different branch or tag, while preserving the existing failure
handling and error reporting.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6a3aa72-12b9-41c3-8e24-ca8a1dc359a4
📒 Files selected for processing (5)
.github/assets/release-body.md.github/workflows/publish.yml.github/workflows/verify-release.ymlREADME.mdcspell.json
| if grep -qF '/blob/v<version>/' <<< "${BODY}"; then | ||
| echo '::error::The release body still has its `/blob/v<version>/` placeholder, so its INSTALL.md link points at a tag that does not exist. Replace `v<version>` with the tag this release is on.' | ||
| failed=1 | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the actual release tag for INSTALL.md.
Line 45 only rejects the literal /blob/v<version>/ placeholder. A manual release can use /blob/main/INSTALL.md or another tag and still pass. The release body is then not pinned to the release that users download. Compare the link with github.event.release.tag_name and fail when the tag does not match.
Suggested validation
env:
BODY: ${{ github.event.release.body }}
ASSET_NAMES: ${{ toJSON(github.event.release.assets.*.name) }}
+ RELEASE_TAG: ${{ github.event.release.tag_name }}
...
+ if ! grep -qF "/blob/${RELEASE_TAG}/INSTALL.md" <<< "${BODY}"; then
+ echo "::error::The release body INSTALL.md link does not target ${RELEASE_TAG}."
+ failed=1
+ fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/verify-release.yml around lines 45 - 48, Update the
release-body validation around the existing placeholder check in the workflow to
extract the `INSTALL.md` blob URL’s tag and compare it with
`github.event.release.tag_name`. Fail the validation when the link uses any
different branch or tag, while preserving the existing failure handling and
error reporting.
The checklist told a maintainer to attach a Studio build without saying where builds come from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Checking only for the unfilled placeholder passed a body copied from the previous release, whose docs link resolves and looks right while serving another version's instructions. Checking for the tag catches that, a reverted `/blob/main/` link, and a deleted link, so the placeholder check it replaces is redundant. Also correct the Studio zip download size: the 0.3.0 Windows installer is 165 MiB, nowhere near the 240 MB both sets of install steps claimed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
I'm getting a 404 on the |
|
The zip that we've uploaded to our releases page is around 240 MB |
|
⛏️ Still says |
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc reviewed 6 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on imnasnainaec).
README.md line 262 at r2 (raw file):
3. Attach the Paratext 10 Studio build to the new draft release. The workflow attaches only the extension zip, so the application build has to be uploaded by hand — attach `Paratext.10.Studio.Setup.<Studio version>-Windows.zip`. Studio builds are published on the [`paratext-10-studio` releases page](https://github.com/paranext/paratext-10-studio/releases). If you left `studioVersion` blank in step 2, also replace `<Studio version>` in the release body's install steps with the version of the zip you attached; the extension zip's own version is always substituted by the workflow. The body has to name the zip you actually attached. The workflow run's job summary records the exact `paranext-core` revision the extension was built against, which is the revision the Studio build should come from. 4. Install both zips and confirm the Interlinearizer tab renders, then adjust the draft release's body and other metadata as desired and publish the release. Until it is published it stays a draft, which is visible only to people with write access to this repo — publishing is what makes the zips downloadable by testers. Publishing also runs the Verify Release workflow, which fails if the body still has an unfilled `<Studio version>`, `interlinearizer_<version>.zip`, or `/blob/v<version>/` placeholder, or if the Studio zip the body names is not among the attached assets; all of these are fixable on the published release, and editing it runs the check again.
This describes the link check as firing on "an unfilled /blob/v<version>/ placeholder", but it actually compares the link against this release's tag — a body copied from a previous release has a fully filled link and still fails. That copied-body case is the motivation the workflow comment gives, so it's worth saying here: the check fails if the INSTALL.md link doesn't point at this release's tag.
.github/workflows/verify-release.yml line 60 at r2 (raw file):
names="$(jq -r '.[]' <<< "${ASSET_NAMES}")" if [ -z "${studio_zip}" ]; then echo '::error::The release body names no Studio zip for testers to download in step 1.'
When studioVersion is left blank — a documented, supported path — this reports a second error that isn't true: the body does name a Studio zip, the extraction regex just can't match across the space in <Studio version>. So the maintainer gets "still has its <Studio version> placeholder" (correct) plus "names no Studio zip for testers to download in step 1" (false), for one root cause.
Tracking the placeholder check in a variable and skipping this block when it fired would leave one accurate error.
The 240 MB figure was right: it describes the zip we upload, not the 165 MiB installer inside it, so the estimate I derived from upstream was wrong. The `paratext-10-studio` link 404s for anyone without access to that private repo, which reads as a broken link rather than a permissions one. Also name the Studio zip placeholder `<Studio version>` in INSTALL.md, matching everywhere else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Swapping the placeholder check for a tag check left step 4 describing a `/blob/v<version>/` placeholder the workflow no longer looks for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc reviewed 3 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on imnasnainaec).
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 2 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on imnasnainaec).
2 participants
Minor tweaks on #208
Devin: https://app.devin.ai/review/sillsdev/interlinearizer-extension/pull/210
This change is
Summary by CodeRabbit
Documentation
Release Improvements