Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Return Command::SUCCESS enum instead of 0 for clarity to newer devs #39154

Merged
merged 1 commit into from
Oct 8, 2021
Merged

[8.x] Return Command::SUCCESS enum instead of 0 for clarity to newer devs #39154

merged 1 commit into from
Oct 8, 2021

Conversation

fylzero
Copy link
Contributor

@fylzero fylzero commented Oct 8, 2021

This question seems to come up with newer devs and I saw this suggestion recently which I think could be useful as a default for the command stub. Swapping the exit status of 0 for the actual command status enum which makes it a bit more clear to a new dev what it actually signifies.

@dennisprudlo
Copy link
Contributor

This change was rejected in a PR from July. #38151

@fylzero
Copy link
Contributor Author

fylzero commented Oct 8, 2021

This change was rejected in a PR from July. #38151

Interesting, I wonder what the reason is. I'm guess because there is already return 0 precedent set in other commands, but doing this for the custom command stub feels like a good idea. I've had even more seasoned devs ask this question or replace it with return true/false, etc. Just think it would be a nice change.

@taylorotwell taylorotwell merged commit 555617e into laravel:8.x Oct 8, 2021
chu121su12 pushed a commit to chu121su12/framework that referenced this pull request Oct 9, 2021
@fylzero fylzero deleted the enum-command-success branch October 12, 2021 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants