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

[Symfony 6] Third solution to fix resource controllers #444

Merged
merged 4 commits into from
Apr 20, 2022

Conversation

loic425
Copy link
Member

@loic425 loic425 commented Apr 20, 2022

Q A
Bug fix? yes (for Symfony 6)
New feature? no
BC breaks? no
Deprecations? no
Related tickets partially #400
License MIT

@loic425 loic425 requested a review from a team as a code owner April 20, 2022 08:16
@loic425 loic425 mentioned this pull request Apr 20, 2022
16 tasks
@loic425 loic425 changed the base branch from 1.10 to maintenance/symfony-6 April 20, 2022 08:17
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

final class TwigPass implements CompilerPassInterface
Copy link
Member

Choose a reason for hiding this comment

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

Let's add note, that this class will be removed in Resource 2.0

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we deprecate this class and inform about removing setting twig as public service in the UPGRADE file?

Copy link
Member Author

@loic425 loic425 Apr 20, 2022

Choose a reason for hiding this comment

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

I think deprecate it should be landed with Sylius resource controller split (next release). Cause we need this usage for this release and can not remove using it.

@lchrusciel lchrusciel added the Bug Confirmed bugs or bugfixes. label Apr 20, 2022
@lchrusciel lchrusciel merged commit af233d7 into Sylius:maintenance/symfony-6 Apr 20, 2022
@lchrusciel
Copy link
Member

Thank you, Loïc! 🥇

@loic425 loic425 deleted the fix/controllers-3 branch April 20, 2022 12:12
lchrusciel added a commit that referenced this pull request Apr 20, 2022
This PR was merged into the 1.10 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Bug fix?        | no
| New feature?    | yes
| BC breaks?      | I hope no
| Deprecations?   | no
| Related tickets | #400
| License         | MIT


Commits
-------

367a014 Add support for Symfony 6
0c2c50c Fix parameters service for Symfony 6
1bf22bc Allow grid bundle symfony 6 branch
a420b52 Fix Psalm errors
43d63f3 Fix PHPStan errors
6e7b7a8 Fix Phpspec for Symfony 6
0c81c2c Commit suggestions
0cb7c9a Apply suggestions from code review
1d165a0 Fix
eb465b8 Simplify tests
73d533d bug #439 Fix Phpspec for symfony 6 (loic425)
dad9a5c Fix wrong error from Psalm
4a3b340 Make Twig public to fix resource controller
c95b968 Fix coding standard
dc22e5b Add a note to remove Twig pass on sylius resource bundle 2.0
af233d7 bug #444  [Symfony 6] Third solution to fix resource controllers (loic425)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bugs or bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants