Stop dumping usage on every failed check - #214
Conversation
A failing check is preflight's normal operating mode, but cobra treats any
error from RunE as a usage mistake, so every failure printed "Error: check
failed" plus the command's full flag list. `preflight env NOPE` produced 30
lines to say one thing, and in a Docker RUN layer or CI log that buries the
message it exists to deliver.
rootCmd sets SilenceUsage and SilenceErrors, and main routes the error
through reportExecuteError: ErrCheckFailed prints nothing further, since
[FAIL] and exit 1 already said it. Everything else — unknown flag, missing
argument, bad flag combination — still prints the message and usage, which
is where usage actually helps.
Uses ExecuteC so a usage error shows the failing subcommand's usage rather
than the root command's.
env NOPE_XYZ 30 lines -> 2
env PATH --nope 27 lines -> 27 (unchanged, as intended)
|
Warning Review limit reached
Next review available in: 55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
==========================================
- Coverage 93.06% 93.00% -0.07%
==========================================
Files 49 49
Lines 1861 1872 +11
==========================================
+ Hits 1732 1741 +9
- Misses 92 94 +2
Partials 37 37
🚀 New features to boost your workflow:
|
A failing check is preflight's normal operating mode. Cobra treats any error from
RunEas a usage mistake, so every failure printedError: check failedplus the command's entire flag list:30 lines to say one thing. In a Docker
RUNlayer or a CI log — where stdout and stderr are merged — five failing checks bury the messages under ~150 lines of flag reference. TheError: check failedline is also pure redundancy;[FAIL]and exit 1 already said it.Change
rootCmdsetsSilenceUsageandSilenceErrors.mainroutes the error throughreportExecuteError, which distinguishes the two cases:ErrCheckFailed→ nothing further. The[FAIL]line and exit 1 are the complete signal.It uses
ExecuteCrather thanExecuteso a usage error shows the failing subcommand's usage, not the root command's.Result
env NOPE_XYZfile /nonexistentcmd definitelynotrealenv PATH --nope(unknown flag)Verified unaffected:
--version,--help, unknown command,runwith a missing.preflight, the exec refusal from #210, and the flag-combination error incmd_json.go— which now correctly shows thejsoncommand's usage.No doc change needed
docs/usage.md's "Output Format" section already showed clean[OK]/[FAIL]blocks with no usage dump. The docs described the intended behavior all along; this makes the binary match them.Tests
TestReportExecuteError, written first and confirmed red, covers: a bareErrCheckFailedprints nothing; a wrappedErrCheckFailedalso prints nothing (so the behavior survives future%wwrapping); a usage error prints both message and usage; and a nil command doesn't panic.