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

Improved autowiring support for templating helpers #217

Merged
merged 1 commit into from
Jun 7, 2018

Conversation

XWB
Copy link
Contributor

@XWB XWB commented Jun 6, 2018

I am targeting this branch, because there should be a next 2.x release with this fix.

Changelog

### Added
- Improved autowiring support for templating helpers

Subject

Trying to auto wire the templating helpers throws the following error:

Autowiring services based on the types they implement is deprecated since Symfony 3.3 and won't be supported in version 4.0. You should rename (or alias) the "sonata.intl.templating.helper.datetime" service to "Sonata\IntlBundle\Templating\Helper\DateTimeHelper" instead.

The solution is rather easy: add templating services based on their FQCN and alias them to the existing services to maintain backwards compatibility.

CHANGELOG.md Outdated
## 2.4.2
### Added
- Improved autowiring support for templating helpers

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't edit this file please, we will do it ourselves when releasing.

<services>
<service id="Sonata\IntlBundle\Templating\Helper\DateTimeHelper" alias="sonata.intl.templating.helper.datetime" />
<service id="Sonata\IntlBundle\Templating\Helper\LocaleHelper" alias="sonata.intl.templating.helper.locale" />
<service id="Sonata\IntlBundle\Templating\Helper\NumberHelper" alias="sonata.intl.templating.helper.number" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on the outcome of this discussion, I think it might be worth renaming the services, and doing the aliasing the other way around: <service id="sonata.intl.templating.helper.datetime" alias="Sonata\IntlBundle\Templating\Helper\DateTimeHelper" />.

I'm not quite sure the current best practice is really that good.

Copy link
Member

Choose a reason for hiding this comment

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

@greg0ire IMO we could refactor this later across all bundles, but this PR looks "helpful" and fixes some DX problems for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. @XWB please fix the build

@XWB XWB force-pushed the autowire-helpers branch 3 times, most recently from 059501b to 17d129d Compare June 7, 2018 10:25
@XWB
Copy link
Contributor Author

XWB commented Jun 7, 2018

@greg0ire All fixed, just the lint test fails due to an unrelated file issue.

@kunicmarko20
Copy link
Contributor

new version of phpcs fixer, just ignore that.

kunicmarko20
kunicmarko20 previously approved these changes Jun 7, 2018
@greg0ire
Copy link
Contributor

greg0ire commented Jun 7, 2018

Related, unrelated, if you want this merged, you will have to either fix it yourself or wait for a fix.

@kunicmarko20
Copy link
Contributor

but in a new PR.

@XWB
Copy link
Contributor Author

XWB commented Jun 7, 2018

CS issue has been fixed in #218

@OskarStark
Copy link
Member

CS issue has been fixed in #218

Thank you @XWB I merged it, please rebase your PR

OskarStark
OskarStark previously approved these changes Jun 7, 2018
@XWB XWB dismissed stale reviews from OskarStark and kunicmarko20 via 526a0a3 June 7, 2018 13:10
@XWB XWB force-pushed the autowire-helpers branch from 17d129d to 526a0a3 Compare June 7, 2018 13:11
@XWB
Copy link
Contributor Author

XWB commented Jun 7, 2018

@OskarStark All done, all green :)

@greg0ire greg0ire merged commit ca945a0 into sonata-project:2.x Jun 7, 2018
@greg0ire
Copy link
Contributor

greg0ire commented Jun 7, 2018

Thanks @XWB !

@XWB XWB deleted the autowire-helpers branch June 7, 2018 19:56
@XWB
Copy link
Contributor Author

XWB commented Jun 15, 2018

@greg0ire @OskarStark Would you guys consider tagging a new release?

@greg0ire
Copy link
Contributor

greg0ire commented Jun 15, 2018

Sure, expect it tomorrow night!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants