refine(cli): rename --exit-zero; fix status_code in cmd_all#187
Merged
Conversation
…n cmd_all - Rename `skip_errors` → `exit_zero` across all four CLI subcommands (check, expiry, chains, all) — `--exit-zero` is the established linter convention (ruff, pylint) and self-describes the semantics; old name read as "swallow Python exceptions" - Fix latent bug in cmd_all: remove `status_code` from `has_errors` OR — cmd_status is documented as always returning 0 and is not a pass/fail gate; including it was dead code today but fragile - Add comment explaining why sub-commands in cmd_all are called with exit_zero=False hard-coded (truthful exit codes needed for aggregate; flag applied once at aggregate level) - Update test method names and call sites accordingly - Update docs (cli.md, audit.md, README.md) to reference --exit-zero --- Co-authored-by: Claude Code <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines the pydeprecate CLI by renaming the “always succeed” flag from skip_errors to exit_zero (documented as --exit-zero) across subcommands, and fixes cmd_all to avoid treating cmd_status as a pass/fail gate when computing aggregate exit status.
Changes:
- Rename
skip_errors→exit_zeroincmd_check,cmd_expiry,cmd_chains, andcmd_all, and update unit tests accordingly. - Fix
cmd_allaggregate error detection to excludecmd_statusfrom determining failure. - Update docs/README to reference
--exit-zeroand clarifycmd_allsubcommand invocation behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/deprecate/_cli.py |
Renames the CLI flag parameter and fixes cmd_all aggregate exit-code logic. |
tests/unittests/test_cli.py |
Updates unit tests to use exit_zero instead of skip_errors. |
README.md |
Updates CLI docs to reference --exit-zero (needs a small correction re: status support). |
docs/guide/cli.md |
Updates CLI reference table/wording to --exit-zero (needs correction re: packaging missing exit behavior for expiry). |
docs/guide/audit.md |
Updates audit guide examples to use --exit-zero. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
The previous SUGGESTION issue has been addressed. The documentation now correctly states that Reviewed by laguna-m.1-20260312:free · 775,256 tokens |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- CHANGELOG [Unreleased] ### Changed: document --skip_errors → --exit-zero rename as breaking change (#187) - docs/guide/cli.md:119 exit-codes table: fix expiry row — packaging-missing now exits 0, not 1; update exit-1 description to 'Expired wrappers found (and --exit-zero not set)' - docs/guide/cli.md:66 expiry tab block: indent prose line 4 spaces so MkDocs Material renders it inside the tab panel (flush-left line broke tab block) - tests/integration/test_cli.py: add subprocess tests confirming both --exit-zero (dash) and --exit_zero (underscore) Fire spellings parse correctly and force exit 0 on a package with invalid args_mapping --- Co-authored-by: Claude Code <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before submitting
What does this PR do?
skip_errors→exit_zeroacross all four CLI subcommands (check, expiry, chains, all) —--exit-zerois the established linter convention (ruff, pylint) and self-describes the semantics; old name read as "swallow Python exceptions"status_codefromhas_errorsOR — cmd_status is documented as always returning 0 and is not a pass/fail gate; including it was dead code today but fragilePR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃