Skip to content

[Console] Add deprecation message for non-int statusCode #33775

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 2, 2019
Merged

[Console] Add deprecation message for non-int statusCode #33775

merged 1 commit into from
Oct 2, 2019

Conversation

jschaedl
Copy link
Contributor

@jschaedl jschaedl commented Sep 30, 2019 •
edited
Loading

Q A
Branch? 4.4
Bug fix? no
New feature? no
Deprecations? yes
Tickets Fix #33747
License MIT
Doc PR -

What was done:

  • added deprecation message for non-int return value in Command::execute()
  • fixed all core commands to return proper int values
  • added proper return type-hint to Command::execute() method in all core Commands

@jschaedl jschaedl mentioned this pull request Sep 30, 2019
Closed
@chalasr chalasr added this to the next milestone Sep 30, 2019
@fabpot
Copy link
Member

fabpot commented Sep 30, 2019

We need to fix the deprecations in this PR (as tests won't pass).

@nicolas-grekas
Copy link
Member

Don't miss updating the corresponding @return docblock also.

@jschaedl jschaedl requested a review from lyrixx as a code owner October 1, 2019 10:22
jschaedl
jschaedl commented Oct 1, 2019
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php
@@ -114,7 +114,7 @@ protected function configure()
/**
* {@inheritdoc}
*/
protected function execute(InputInterface $input, OutputInterface $output)
protected function execute(InputInterface $input, OutputInterface $output): int
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ContainerDebugCommand is marked as @internal and all subclasses already introduced an int return type-hint for execute(). So I think it is save to add a return type here as well.

@jschaedl jschaedl force-pushed the issue-33747-1 branch 5 times, most recently from 603ad9e to 3647ef5 Compare October 1, 2019 21:13
@jschaedl jschaedl mentioned this pull request Oct 2, 2019
Merged
xabbuh added a commit that referenced this pull request Oct 2, 2019
@xabbuh
bug #33805 [FrameworkBundle] Fix wrong returned status code in Config…
fb5f8fb 
…DebugCommand (jschaedl)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Fix wrong returned status code in ConfigDebugCommand

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #33747<!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | -

This is a follow-up PR caused by #33775 (comment)

Commits
-------

9b5ced2 [FrameworkBundle] Fix wrong returned status code in ConfigDebugCommand
@jschaedl @nicolas-grekas
[Console] Command::execute() should always return int - deprecate ret…
98c4f6a 
…urning null

- added deprecation message for non-int return value in Command::execute()
- fixed all core commands to return proper int values
- added proper return type-hint to Command::execute() method in all core Commands
@nicolas-grekas
Copy link
Member

Thank you @jschaedl.

nicolas-grekas added a commit that referenced this pull request Oct 2, 2019
@nicolas-grekas
feature #33775 [Console] Add deprecation message for non-int statusCo…
c2111be 
…de (jschaedl)

This PR was merged into the 4.4 branch.

Discussion
----------

[Console] Add deprecation message for non-int statusCode

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #33747 <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | -

### What was done:

- [x] added deprecation message for non-int return value in Command::execute()
- [x] fixed all core commands to return proper int values
- [x] added proper return type-hint to Command::execute() method in all core Commands

Commits
-------

98c4f6a [Console] Command::execute() should always return int - deprecate returning null
@nicolas-grekas nicolas-grekas merged commit 98c4f6a into symfony:4.4 Oct 2, 2019
This was referenced Oct 4, 2019
Merged
Merged
@TomasVotruba TomasVotruba mentioned this pull request Oct 13, 2019
Merged
@derrabus derrabus mentioned this pull request Oct 13, 2019
Closed
3 tasks
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
Merged
Merged
@odolbeau odolbeau mentioned this pull request Nov 23, 2019
Merged
@jschaedl jschaedl mentioned this pull request Nov 27, 2019
Merged
@XWB XWB mentioned this pull request Nov 27, 2019
Merged
@jschaedl jschaedl deleted the issue-33747-1 branch February 23, 2020 08:02
@Taluu Taluu mentioned this pull request Nov 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@stof stof stof left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@lyrixx lyrixx Awaiting requested review from lyrixx lyrixx is a code owner

Assignees
No one assigned
Labels
Console Deprecation Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.

Command::execute() should always return int - deprecate returning null
6 participants
@jschaedl @fabpot @nicolas-grekas @stof @chalasr @carsonbot