Problem/Motivation

Documentation for Symfony\Component\Console\Command\Command::execute() indicates that the method should return an integer, specifically 0 on success or an error code.

Drupal\Core\Command\DbImportCommand::execute()
Drupal\Core\Command\DbDumpCommand::execute()
and
Drupal\Tests\system\Kernel\Scripts\DbCommandBaseTester::execute()
do not return anything at all, but should return an integer.

Additionally, in Symfony 4.4, this lack of return value will trigger a deprecation warning.

Proposed resolution

Add the return value to these methods.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3085608-8.patch4.8 KBwim leers
#8 interdiff.txt3.67 KBwim leers
#2 3085608-2.drupal.Symfony-console-command-execute-methods-should-return-0-on-success.patch1.6 KBmikelutz

Comments

mikelutz created an issue. See original summary.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Active » Needs review
StatusFileSize
new 3085608-2.drupal.Symfony-console-command-execute-methods-should-return-0-on-success.patch1.6 KB

Set to major, because this should be in prior to branching 9.0.x

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Title: Symfony console command ::execute() methods should return 0 on success » [Symfony 4] Symfony console command ::execute() methods should return 0 on success
Assigned: mikelutz » Unassigned
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Issue summary: View changes
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

Are those ones ok?

  • composer/Plugin/Scaffold/ComposerScaffoldCommand.php
  • core/lib/Drupal/Core/Command/GenerateProxyClassCommand.php
  • core/tests/Drupal/TestSite/Commands/TestSiteInstallCommand.php
  • core/tests/Drupal/TestSite/Commands/TestSiteReleaseLocksCommand.php
  • core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php
  • core/tests/Drupal/TestSite/Commands/TestSiteUserLoginCommand.php
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented

They probably should be fixed if they don't return an integer, though they did not trigger an error in the test run.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Needs work

Yes, lets fix all of them in one go - thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new interdiff.txt3.67 KB
new 3085608-8.patch4.8 KB

Updated those too. Thanks, @voleger!

Since this is just me repeating a trivial pattern, and I'd have RTBC'd #2, I think in this case self-RTBC'ing is acceptable.

(Especially because this blocks #2976394: Allow Symfony 4.4 to be installed in Drupal 8, which is super important for Drupal 9.)

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

+1 for RTBC

  • catch committed 1e6ffcb on 8.8.x 
    Issue #3085608 by Wim Leers, mikelutz, voleger, larowlan: [Symfony 4]...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 1e6ffcb and pushed to 8.8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.