fix: exit with success command line status 0 for not implemented things #730
Conversation
This file contains 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
|
|
@jvillafanez @IljaN @jnweiger if anyone is back from leave, please review. |
jnweiger
approved these changes
Jan 9, 2024
There was a problem hiding this comment.
The "not implemented" messages were probably originally meant to abort.
But yeah, backwards compatibility "keep going despite the 'errors'" is more important.
Maybe add a comment that we intend to continue. Or change the messages to something like
$output->writeln('upgrade:backupDb command is not implemented. Continuing anyway...');
But that is only eye-candy.
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.
PR #711 made sure to return an
intcommand line status from all Symfony commandexecutefunctions.But it returned
1for commands that are not yet implemented. That causes scripts to fail if they call any of the not-yet-implemented commands. That has resulted in upgrades failing when they previously worked (albeit with some steps not implemented). Although some steps were not yet implemented, the actual upgrade was working.This PR changes the command line status to
0for not-yet-implemented things.0is success for the command line. That puts the behavior back to the way it was.See discussion in issue #726
Also, remove phpstan ignoreErrors entries for things that are not errors, Recent versions of phpstan no longer have these false positives, so they are no longer needed in
phpstan.neon- this gets phpstan passing in CI.