Conversation
📝 WalkthroughWalkthroughThis PR bumps the package version from 6.1.1 to 6.1.2 in package.json and src/lib/constants.ts, and enhances the release workflow with retry logic for NPX and bunx install path verification steps, each attempting up to 6 times with 10-second intervals before failing. Changes
Possibly related PRs
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
85-95: Extract shared retry logic to avoid drift betweennpxandbunxchecks.These blocks are structurally identical; moving them into one helper in each
runscript keeps behavior consistent and lowers maintenance risk.♻️ Example refactor
- name: Verify npx install path run: | VERSION="$(node -p "require('./package.json').version")" - EXPECTED="TTDash v${VERSION}" - for attempt in 1 2 3 4 5 6; do - OUTPUT="$(npm exec --yes --package "@roastcodes/ttdash@${VERSION}" -- ttdash --help 2>&1)" && { - printf '%s\n' "$OUTPUT" - printf '%s\n' "$OUTPUT" | grep -F "$EXPECTED" >/dev/null - exit 0 - } - sleep 10 - done - echo "npm exec install path did not become ready in time." - exit 1 + EXPECTED="TTDash v${VERSION}" + verify_install() { + local cmd="$1" + local label="$2" + for attempt in 1 2 3 4 5 6; do + OUTPUT="$(eval "$cmd" 2>&1)" && { + printf '%s\n' "$OUTPUT" + printf '%s\n' "$OUTPUT" | grep -F "$EXPECTED" >/dev/null && exit 0 + } + sleep 10 + done + echo "${label} install path did not become ready in time." + exit 1 + } + verify_install 'npm exec --yes --package "@roastcodes/ttdash@'"${VERSION}"'" -- ttdash --help' 'npm exec'Also applies to: 100-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 85 - 95, The retry loop that runs the check (variables EXPECTED and OUTPUT, the for-attempt loop, the npm exec "--package "@roastcodes/ttdash@${VERSION}" -- ttdash --help" invocation, the grep check and exit logic) is duplicated for different runners (npx/bunx/npm exec); extract that shared retry logic into a single reusable helper function or script invoked by each run step so both checks call the same implementation; implement a helper (e.g., retry_check or run_with_retries) that accepts the command to run and the expected string, reuse it in the run blocks instead of copying the for loop, and ensure it preserves the same sleep, attempts, exit codes, and output printing behavior.src/lib/constants.ts (1)
1-1: Consider eliminating dual version sources.
server.jsreads runtime version frompackage.json(Line 12), while this constant is maintained separately. Keeping both can drift over time; consider deriving UI/library version from one source (or enforcing equality in CI).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/constants.ts` at line 1, The file exports a hardcoded VERSION constant (export const VERSION) which duplicates the runtime version pulled from package.json in server.js; replace the hardcoded value by deriving it from package.json (e.g., import or require package.json at build/runtime or generate constants.ts during the build) so there is a single source of truth, or alternatively add a CI check that asserts equality between package.json's version and the VERSION export; update the symbol VERSION in src/lib/constants.ts accordingly and ensure any consumers still import VERSION as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 85-95: The retry loop that runs the check (variables EXPECTED and
OUTPUT, the for-attempt loop, the npm exec "--package
"@roastcodes/ttdash@${VERSION}" -- ttdash --help" invocation, the grep check and
exit logic) is duplicated for different runners (npx/bunx/npm exec); extract
that shared retry logic into a single reusable helper function or script invoked
by each run step so both checks call the same implementation; implement a helper
(e.g., retry_check or run_with_retries) that accepts the command to run and the
expected string, reuse it in the run blocks instead of copying the for loop, and
ensure it preserves the same sleep, attempts, exit codes, and output printing
behavior.
In `@src/lib/constants.ts`:
- Line 1: The file exports a hardcoded VERSION constant (export const VERSION)
which duplicates the runtime version pulled from package.json in server.js;
replace the hardcoded value by deriving it from package.json (e.g., import or
require package.json at build/runtime or generate constants.ts during the build)
so there is a single source of truth, or alternatively add a CI check that
asserts equality between package.json's version and the VERSION export; update
the symbol VERSION in src/lib/constants.ts accordingly and ensure any consumers
still import VERSION as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf9d4fe7-2e6f-4959-86a0-4e1939995bec
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.github/workflows/release.ymlpackage.jsonsrc/lib/constants.ts
Summary by CodeRabbit
Release Notes