Prepare 4.7.2 release and fix packaging failure exit status - #252
Merged
Merged
Conversation
The package command caught every exception, printed "Error: ..." to stdout and returned normally, so the process exited 0. Callers could not tell a failed run from a successful one: CI moved on to the integration tests, and flet build continued past the failed step, committed its package hash (so the next build passes --skip-site-packages) and, outside verbose mode, never showed the captured message. The catch writes the error to stderr and sets exitCode = 1. It uses exitCode rather than exit() so the finally block still removes the temporary directory and stops the Pyodide PyPI server. The early exit(1) and exit(2) paths are unchanged.
ndonkoHenri
requested review from
FeodorFitsner
and
a balanced review from Copilot
September 24, 2026 07:29
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The reviewed changes correctly report packaging failures, with no unresolved issues.
Review effort: Balanced
Findings: None
What changed in this PR
Ensures packaging exceptions return a non-zero status while preserving cleanup behavior.
Changes:
- Writes caught packaging errors to stderr and sets
exitCode = 1. - Documents the fix in the 4.7.2 changelog.
| File | Description |
|---|---|
src/serious_python/CHANGELOG.md |
Adds the 4.7.2 release note. |
src/serious_python/bin/package_command.dart |
Reports caught packaging failures with status 1 while preserving cleanup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
FeodorFitsner
changed the title
FeodorFitsner
approved these changes
Sep 24, 2026
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.
Packaging exceptions currently print an error but leave the command's exit status at 0. CI and
flet buildcan therefore continue after failed packaging;flet buildmay also cache the failed step as complete. Write these errors to stderr and setdart:io'sexitCode = 1, allowing the existingfinallycleanup to run.This PR also prepares all six packages for the 4.7.2 release:
main, including fix(darwin): re-sign linker-signed macOS native modules while staging #251, and combine the 4.7.2 changelog entries.flutter pub get --offline; commit the two tracked lockfiles. The bridge example lockfile remains ignored by repository policy.Validation with Flutter 3.44.8:
flutter test --no-pub test: passed.dart analyze bin: no issues.git diff --check: passed.SERIOUS_PYTHON_APPexits 1 with stderr output and temporary-directory cleanup; successful native staging exits 0 and cleans up.Full platform integration coverage runs in CI. Existing explicit
exit(1)/exit(2)paths are unchanged. No release tag or publication is part of this PR.