Skip to content

feat: allow symfony/console 8#75

Merged
mnapoli merged 1 commit into
mnapoli:masterfrom
canvural:symfony8
May 16, 2026
Merged

feat: allow symfony/console 8#75
mnapoli merged 1 commit into
mnapoli:masterfrom
canvural:symfony8

Conversation

@canvural
Copy link
Copy Markdown
Contributor

This PR allows symfony/console version 8. And upgrades the PHPCSFixer to v3

@canvural canvural marked this pull request as ready for review May 16, 2026 15:16
@canvural canvural force-pushed the symfony8 branch 5 times, most recently from a9f09af to 75ee0df Compare May 16, 2026 15:38
@mnapoli
Copy link
Copy Markdown
Owner

mnapoli commented May 16, 2026

Thank you!

(btw I wouldn't be against dropping support for older versions of PHP and Symfony)

@mnapoli mnapoli merged commit c0ceb71 into mnapoli:master May 16, 2026
7 checks passed
@canvural canvural deleted the symfony8 branch May 16, 2026 19:49
@canvural
Copy link
Copy Markdown
Contributor Author

Noted. I'm reviving a project that uses Silly from 2 years ago. I might send more PRs.

@shadowhand
Copy link
Copy Markdown

This broke my existing applications. It does more than just allow Symfony v8, it mandates a int return from all my commands. Previously, Silly would assume that a void|null return meant return 0.

@mnapoli
Copy link
Copy Markdown
Owner

mnapoli commented May 18, 2026

@shadowhand I don't understand which part of the diff does that, can you point it out?

Comment thread src/Application.php

$this->add($command);
if (method_exists($this, 'addCommand')) {
$this->addCommand($command);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This breaks any command that doesn't return an int.

You can tell because the author had to add return 0 all over the place.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you that's more helpful. @canvural would you be able to look into that?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fix in #77

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