feat(status): add exit-code flag - #563
Conversation
|
🚀 Website Preview Your PR preview is ready! 📎 Preview URL: https://jongio.github.io/azd-app/pr/563/ This preview will be automatically cleaned up when the PR is closed. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #563 +/- ##
==========================================
+ Coverage 61.83% 61.89% +0.05%
==========================================
Files 224 224
Lines 29418 29429 +11
==========================================
+ Hits 18192 18216 +24
+ Misses 9978 9962 -16
- Partials 1248 1251 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
wbreza
left a comment
There was a problem hiding this comment.
Code Review — Approve ✅
Summary: Adds an --exit-code flag to azd app status that returns a non-zero exit code when the app runner is not active. Clean, well-scoped, and well-tested (2 files, ~30 lines). The primary scripting/CI use case is correct: not-running → exit 1, running → exit 0.
What I liked:
- The JSON path prints the report before returning the exit-code error, so machine-readable output stays clean while still signaling status via the exit code. Nice separation.
- The
statusExitCodeErrorhelper is small, pure, and the test covers all three branches (flag off, flag on + running, flag on + not running) plus flag registration. - Error message routes through the existing
main.goerror path, yieldingos.Exit(1).
Non-blocking observations (Low):
- Redundant messaging in text mode. When not running with
--exit-code, the user seesApp is not runningon stdout andapp runner is not activeon stderr (two phrasings of the same state). Functionally fine; consider reusing a single phrasing for polish. --watch+--exit-codeinteraction is silent. In text mode,--watchreturns beforestatusExitCodeErroris reached, so--exit-codeis silently ignored (watch is long-lived/interactive, so this is reasonable — just undocumented). Worth a one-line note in help text or a docs mention.
Neither observation blocks merge. Approving.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e02217cd-ffed-496e-b8ea-848a781cda64
9d5dde2 to
7b2796d
Compare
Agreed — fixed in
So stdout and stderr now describe the same state with the same phrasing, and the wording is consistent with the rest of To keep the two from drifting apart again, |
wbreza
left a comment
There was a problem hiding this comment.
Code Review (re-review) — Approve ✅
Re-reviewed at HEAD 7b2796d (new commit since my prior approval on 9d5dde2). The update rebases onto newer main (which added --dashboard-url) and addresses both non-blocking notes from my last pass:
- ✅ Consistent wording — the exit-code error is now
"app is not running", aligned with the text report, and a test asserts the exact message. - ✅ Documented
--watchinteraction — the Long help and flag usage now state--exit-codeis ignored with--watch("ignored with --watch"/ "It is ignored by the interactive --watch view").
Verification:
- JSON path still prints the report before applying the exit-code error, keeping machine output clean.
statusExitCodeErrorremains a pure helper; test covers all three branches (flag off, on+running, on+not-running) plus flag registration.--watchreturns before the exit-code check, matching the now-documented behavior.
Trivial (non-blocking): the code comment says the error "matches the text report … the same way," but the display string is App is not running (capitalized) vs the error app is not running (lowercase). The lowercase error is correct Go convention — no change needed; just noting the comment is slightly loose on casing.
Zero Critical/High/Medium findings. Approving.
Agreed — documented in Kept the behavior as-is for exactly the reason you gave: Flag help: Long help:
The phrasing follows the existing One note on scope: this only applies to the interactive text view. With the global |
Rebase + CI noteRebased onto
Local validation on the rebased tree:
On the red Windows check: the failure was |
🚀 Test This PRA preview build ( 🌐 Website PreviewLive Preview: https://jongio.github.io/azd-app/pr/563/ One-Line Install (Recommended)PowerShell (Windows): iex "& { $(irm https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/install-pr.ps1) } -PrNumber 563 -Version 0.20.0-pr563"Bash (macOS/Linux): curl -fsSL https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/install-pr.sh | bash -s 563 0.20.0-pr563UninstallWhen you're done testing: PowerShell (Windows): iex "& { $(irm https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/uninstall-pr.ps1) } -PrNumber 563"Bash (macOS/Linux): curl -fsSL https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/uninstall-pr.sh | bash -s 563Build Info:
What to Test: |
Good catch, and you're right on both counts — thanks for calling it out precisely. To confirm the intent for anyone reading later: the casing difference is deliberate, not an oversight. Agreed the comment's "the same way" is loose about that. Taking your "no change needed" at face value and leaving it as-is rather than spending another full CI cycle and review round on a five-word comment tweak on an approved, green PR. Happy to fold the tightening into the next change that touches this file. |
The 15 commits that landed on main while this branch was open added CLI surface the docs gate correctly rejected. Resolve the cli-reference.md conflict by keeping both sides, then backfill everything the gate found. Commands documented: - open (#552), including cli/docs/commands/open.md and the URL resolution order - website reference pages for hooks (#515) and remove (#511), which shipped without generated pages Flags documented: - run --env (#498), run --no-deps (#544) - logs --summary (#491), logs --min-level (#527), logs --no-timestamps (#561) - status --exit-code (#563) - env --prefix (#490) Also fix the gofumpt failure in env_test.go that currently breaks mage preflight on main. mage preflight passes clean. Docs gate covers 31 commands, website validation covers 27. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds
azd app status --exit-codeso scripts can fail when the app runner is not active while keeping normal status output.Validation:
go build ./...go test ./...Closes #560