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.

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

Make TemplateRegistry not mutable #6556

Merged
merged 1 commit into from
Nov 4, 2020
Merged

Make TemplateRegistry not mutable #6556

merged 1 commit into from
Nov 4, 2020

Conversation

wbloszyk
Copy link
Member

@wbloszyk wbloszyk commented Oct 31, 2020 •
edited
Loading

Subject

Global templates should not be changeable in global template registry after created. This PR will add NEXT_MAJOR for do it.
Pool should have TemplateRegistryInterface.
Admin should have MutableTemplateRegistryInterface.

I am targeting this branch, because this change respect BC.

Changelog

### Added
- Added `Sonata\AdminBundle\Templating\AbstractTemplateRegistry`
- Added `Sonata\AdminBundle\Templating\MutableTemplateRegistry` 
- Added `Sonata\AdminBundle\Templating\MutableTemplateRegistryAwareInterface`
### Changed
- Changed `Sonata\AdminBundle\Templating\TemplateRegistryAwareInterface` to handle instances of `TemplateRegistryInterface`.
### Deprecated
- Deprecated `Sonata\AdminBundle\Templating\TemplateRegistry:setTemplate()`
- Deprecated `Sonata\AdminBundle\Templating\TemplateRegistry:setTemplates()`

@wbloszyk wbloszyk force-pushed the fix_template_registry branch 2 times, most recently from 2475673 to 505800d Compare October 31, 2020 18:15
@wbloszyk wbloszyk requested review from VincentLanglet, phansys and a team October 31, 2020 18:21
@wbloszyk wbloszyk mentioned this pull request Oct 31, 2020
Closed
@wbloszyk wbloszyk force-pushed the fix_template_registry branch from 505800d to 019890c Compare November 1, 2020 12:08
@wbloszyk wbloszyk requested review from greg0ire and a team November 1, 2020 12:12
@wbloszyk wbloszyk force-pushed the fix_template_registry branch 2 times, most recently from dc90904 to 2c98688 Compare November 1, 2020 12:37
@wbloszyk wbloszyk requested review from VincentLanglet and a team November 1, 2020 12:37
greg0ire
greg0ire previously approved these changes Nov 1, 2020
View reviewed changes
phansys
phansys requested changes Nov 1, 2020
View reviewed changes
src/Resources/config/core.php Outdated Show resolved Hide resolved
src/Templating/BaseTemplateRegistry.php Outdated Show resolved Hide resolved
src/Templating/MutableTemplateRegistry.php Show resolved Hide resolved
@wbloszyk wbloszyk force-pushed the fix_template_registry branch 2 times, most recently from d42b8a4 to f92ed1a Compare November 1, 2020 16:34
@wbloszyk wbloszyk requested review from greg0ire, phansys, VincentLanglet and a team November 1, 2020 16:38
@VincentLanglet
Copy link
Member

You forgot to add a trigger_error in TemplateRegistry:setTemplate and TemplateRegistry::setTemplates.

And do we need an upgrade note ?

@wbloszyk
Copy link
Member Author

wbloszyk commented Nov 2, 2020

IMO we do not need upgrade note. All change will be resolve by remove deprecations. Or I miss something?
WDYT? @phansys

@VincentLanglet
Copy link
Member

IMO we do not need upgrade note. All change will be resolve by remove deprecations. Or I miss something?

Deprecation is not always easy to catch for developers.
It's easier to read the upgrade note in order to know what to do to avoid the deprecation.

BTW, tests are failing now

@wbloszyk
Copy link
Member Author

wbloszyk commented Nov 2, 2020

Tomorrow I have free day. I can work on this and setTemplate issue. I will add upgrade note too.

phansys
phansys requested changes Nov 2, 2020
View reviewed changes
src/Templating/TemplateRegistry.php Outdated Show resolved Hide resolved
src/Templating/TemplateRegistry.php Outdated Show resolved Hide resolved
phansys
phansys requested changes Nov 3, 2020
View reviewed changes
src/Templating/MutableTemplateRegistry.php Show resolved Hide resolved
src/Templating/MutableTemplateRegistryAwareInterface.php Outdated Show resolved Hide resolved
src/Templating/TemplateRegistry.php Outdated Show resolved Hide resolved
tests/Templating/MutableTemplateRegistryTest.php Outdated Show resolved Hide resolved
tests/Templating/MutableTemplateRegistryTest.php Show resolved Hide resolved
tests/Templating/MutableTemplateRegistryTest.php Outdated Show resolved Hide resolved
tests/Templating/MutableTemplateRegistryTest.php Outdated Show resolved Hide resolved
tests/Templating/TemplateRegistryTest.php Outdated Show resolved Hide resolved
@wbloszyk wbloszyk force-pushed the fix_template_registry branch from 3874c37 to e4637fa Compare November 3, 2020 14:25
@wbloszyk wbloszyk requested review from phansys and a team November 3, 2020 14:25
@wbloszyk wbloszyk force-pushed the fix_template_registry branch 2 times, most recently from 1991a89 to fd96314 Compare November 3, 2020 18:58
@wbloszyk wbloszyk force-pushed the fix_template_registry branch from b33d68e to b08a7db Compare November 3, 2020 20:37
@wbloszyk wbloszyk force-pushed the fix_template_registry branch from b08a7db to ae4c461 Compare November 3, 2020 21:08
@wbloszyk wbloszyk requested review from phansys and a team November 3, 2020 21:09
VincentLanglet
VincentLanglet previously approved these changes Nov 3, 2020
View reviewed changes
phansys
phansys reviewed Nov 3, 2020
View reviewed changes
Copy link
Member

@phansys phansys left a comment

Choose a reason for hiding this comment

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

I'll be reviewing the upgrade notes.

phansys
phansys requested changes Nov 3, 2020
View reviewed changes
UPGRADE-3.x.md Outdated Show resolved Hide resolved
UPGRADE-3.x.md Outdated Show resolved Hide resolved
UPGRADE-3.x.md Outdated Show resolved Hide resolved
UPGRADE-3.x.md Outdated Show resolved Hide resolved
UPGRADE-3.x.md Outdated Show resolved Hide resolved
UPGRADE-3.x.md Outdated Show resolved Hide resolved
UPGRADE-3.x.md Outdated Show resolved Hide resolved
@wbloszyk @phansys
Make TemplateRegistry not mutable
362c01f 
Update UPGRADE-3.x.md

Co-authored-by: Javier Spagnoletti <[email protected]>
@wbloszyk wbloszyk force-pushed the fix_template_registry branch from 79d81f4 to 362c01f Compare November 4, 2020 09:08
phansys
phansys reviewed Nov 4, 2020
View reviewed changes
Copy link
Member

@phansys phansys left a comment

Choose a reason for hiding this comment

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

We're almost done.
Could you please add the changelog section in the description?

@wbloszyk wbloszyk requested a review from phansys November 4, 2020 09:59
@VincentLanglet VincentLanglet merged commit 3960403 into sonata-project:3.x Nov 4, 2020
@VincentLanglet
Copy link
Member

Thanks

@wbloszyk wbloszyk deleted the fix_template_registry branch November 4, 2020 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VincentLanglet VincentLanglet VincentLanglet approved these changes

@phansys phansys phansys approved these changes

@greg0ire greg0ire Awaiting requested review from greg0ire

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@wbloszyk @VincentLanglet @phansys @SonataCI @greg0ire