-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Rename !tagged to !tagged_iterator #31289
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
Comments
makes sense |
I'd like to work on this. |
If I'm missing something, please excuse me, but won't this complicate definition of services for no real benefit, if one would want to support multiple versions of Symfony and at the same time fix deprecations? In my opinion, it is kinda similar to what happened to deprecating single colon notation for route controllers, which was later reversed, due to complicating support for multiple Symfony versions. |
@emodric that's a valid point to consider. Thinking about it:
With this analysis in mind, I'm on the side the objection shouldn't prevent the improvement from happening. Makes sense? |
Maybe. But the whole point of using But okay, it's true that in this case, there's a migration path. |
…or (jschaedl) This PR was squashed before being merged into the 4.4 branch (closes #31321). Discussion ---------- [DI] deprecates tag !tagged in favor of !tagged_iterator | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #31289 | License | MIT | Doc PR | tbd. ### Todo - [x] fix tests Commits ------- ab8fb18 [DI] deprecates tag !tagged in favor of !tagged_iterator
…terator (jschaedl) This PR was merged into the 5.0-dev branch. Discussion ---------- [DI] remove deprecated tag !tagged in favor of !tagged_iterator | Q | A | ------------- | --- | Branch? | master <!-- see below --> | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | yes <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | related ticket #31289 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#tbd <!-- required for new features --> This PR removes tag `tagged` which was deprecated in #31321 Commits ------- 9978184 [DI] removed tagged
@nicolas-grekas Can you give a few pointers on how to replace |
This is the part that should be dropped on your side: don't use the syntax, use a regular argument instead and you'd do in SF2 |
I can't do that :/ The code in question is not a standalone app, but a product that can be installed on multiple apps (eZ Platform, Sylius) with multiple versions of Symfony (currently 3.4 and 4.3), but in 6 months it will be 3.4 (lts), 4.4 (lts) and 5.0. This is the reason for my original comments. There's no way to programatically get rid of the deprecation and still support multiple Symfony versions (especially LTS ones) and I'd rather not reintroduce compiler passes just to injected tagged services which I already happily removed before when I dropped support for Symfony 2.8. |
For those facing a similar issue, I found a non-intrusive way (meaning, it is only ran once, on container compile) by overriding <?php
declare(strict_types=1);
namespace MyApp;
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader as BaseYamlFileLoader;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\Yaml\Tag\TaggedValue;
final class YamlFileLoader extends BaseYamlFileLoader
{
protected function loadFile($file): array
{
$content = parent::loadFile($file);
if (Kernel::VERSION_ID < 40400 || !array_key_exists('services', $content)) {
return $content;
}
foreach ($content['services'] as $serviceId => $service) {
foreach ($service['arguments'] ?? [] as $key => $argument) {
if ($argument instanceof TaggedValue && $argument->getTag() === 'tagged') {
$content['services'][$serviceId]['arguments'][$key] = new TaggedValue('tagged_iterator', $argument->getValue());
}
}
}
return $content;
}
} |
In #30348 we introduced
!tagged_locator
, that complements the previously existing!tagged
.I feel like it would be more clear to rename
!tagged
to!tagged_iterator
.WDYT? Anyone willing to give it a try? That'd mean triggering a deprecation when
!tagged
is used.The text was updated successfully, but these errors were encountered: