Updates the maintenance command output#10070
Conversation
Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
|
Thanks for your pull request.
Regarding the DI, we can probably move a lot of the static code calls there, but I would recommend to tackle that in a separate PR. |
juliusknorr
added
3. to review
|
Acceptance test failures seem unrelated to me. |
MorrisJobke
left a comment
MorrisJobke
left a comment
There was a problem hiding this comment.
Tested, works and code looks good 👍
| $this->config->setSystemValue('maintenance', true); | ||
| $output->writeln('Maintenance mode enabled'); | ||
| } else { | ||
| $output->writeln('Maintenance mode already enabled'); |
There was a problem hiding this comment.
I think I wouldn't change this, to not cause problems with scripts.
There was a problem hiding this comment.
Scripts should not evaluate the content that strictly, but rather use exit code (because they are there for that ;))
There was a problem hiding this comment.
Let me add it to the list of changes for 14.
There was a problem hiding this comment.
Well there is no exit code, so no matter if something went wrong or not, it's always the same?
Anyway changelist and go on 👍
There was a problem hiding this comment.
Well there is no exit code, so no matter if something went wrong or not, it's always the same?
Then this is a different problem. :/
Yes - they are unrelated. |
4 participants
See the commit messages for the main changes.
I've had some trouble adding a test for the Console/Application. It lacks dependency injection and even does a
require once. Running theboostrap.phpfor tests even didn't help. If anyone has a tip on how this can be done, I'd be happy to know.This PR fixes #9949 and fixes #8399 .