Skip to content

[3.0] remove the most blocking return types#3156

Merged
fabpot merged 1 commit into
twigphp:3.xfrom
nicolas-grekas:no-return-type
Sep 20, 2019
Merged

[3.0] remove the most blocking return types#3156
fabpot merged 1 commit into
twigphp:3.xfrom
nicolas-grekas:no-return-type

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented Sep 20, 2019

Copy link
Copy Markdown
Contributor

These return types make it needlessly hard to move to Twig 3.
This should make Symfony 4.4 compatible with Twig 3, and should help make the CI of Symfony 5 green.

Declaration of Twig\Extra\CssInliner\CssInlinerExtension::getFilters() must be compatible with Twig\Extension\AbstractExtension::getFilters(): array in /home/travis/build/symfony/symfony/vendor/twig/cssinliner-extra/src/CssInlinerExtension.php

@nicolas-grekas nicolas-grekas force-pushed the no-return-type branch 4 times, most recently from 9e44b92 to e6bcf51 Compare September 20, 2019 12:23
Comment thread composer.json Outdated
@fabpot

fabpot commented Sep 20, 2019

Copy link
Copy Markdown
Contributor

Thank you @nicolas-grekas.

fabpot added a commit that referenced this pull request Sep 20, 2019
…ekas)

This PR was merged into the 3.x branch.

Discussion
----------

[3.0] remove the most blocking return types

These return types make it needlessly hard to move to Twig 3.
This should make Symfony 4.4 compatible with Twig 3, and should help make the CI of Symfony 5 green.
> `Declaration of Twig\Extra\CssInliner\CssInlinerExtension::getFilters() must be compatible with Twig\Extension\AbstractExtension::getFilters(): array in /home/travis/build/symfony/symfony/vendor/twig/cssinliner-extra/src/CssInlinerExtension.php`

Commits
-------

ac0f512 [3.0] remove the most blocking return types
@fabpot fabpot merged commit ac0f512 into twigphp:3.x Sep 20, 2019
@nicolas-grekas nicolas-grekas deleted the no-return-type branch September 20, 2019 13:08
@stof

stof commented Sep 20, 2019

Copy link
Copy Markdown
Member

Why are they making it hard ? Couldn't Symfony 4.4 have return types on these methods ?

@nicolas-grekas

Copy link
Copy Markdown
Contributor Author

That'd be a BC break for Symfony.

@stof

stof commented Sep 20, 2019

Copy link
Copy Markdown
Member

Well, then we will need a 4.0 version of Twig to get return types on extension points just because we want to support Twig 3 and SF 4.4 and our Twig extensions were not final yet ?

@nicolas-grekas

Copy link
Copy Markdown
Contributor Author

Because the ecosystem deserves it. There are extensions into the wild and adding return types is asking everyone to work or die. We can and should do friendlier. What this PR provides.

fabpot added a commit to symfony/symfony that referenced this pull request Sep 23, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

Allow Twig 3

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Needs twigphp/Twig#3156
And twigphp/Twig#3158

Commits
-------

09f4eb5 Allow Twig 3
symfony-splitter pushed a commit to symfony/http-kernel that referenced this pull request Sep 23, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

Allow Twig 3

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Needs twigphp/Twig#3156
And twigphp/Twig#3158

Commits
-------

09f4eb5 Allow Twig 3
@smnandre

Copy link
Copy Markdown
Contributor

Sorry to digg that far... should we add those return types back now.. if we want to add them in Twig 4.0 ?

@fabpot

fabpot commented Aug 17, 2024

Copy link
Copy Markdown
Contributor

I suppose so @smnandre
Do you want to work on this?

@stof

stof commented Aug 17, 2024

Copy link
Copy Markdown
Member

We kept this in phpdoc, so the Symfony DebugClassLoader triggers deprecations since years if you don't add them in your child classes.
the only remaining work is to add them back in the 4.x branch

@smnandre

Copy link
Copy Markdown
Contributor

I suppose so @smnandre Do you want to work on this?

With pleasure!

the only remaining work is to add them back in the 4.x branch

Ok so i "revert" this commit on the 4.0 branch.. or is there more to do / some tricks along the way ? :)

@fabpot

fabpot commented Aug 17, 2024

Copy link
Copy Markdown
Contributor

I suppose so @smnandre Do you want to work on this?

With pleasure!

the only remaining work is to add them back in the 4.x branch

Ok so i "revert" this commit on the 4.0 branch.. or is there more to do / some tricks along the way ? :)

It should be that simple… let’s see if that's the case in practice 🧐

fabpot added a commit that referenced this pull request Aug 17, 2024
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Add missing return types in 4.0

Revert the removal of return types done in #3156

Should i propagate to `extra` packages in the same PR ?

Commits
-------

470965f Add missing return types in 4.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants