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

[DI] deprecates tag !tagged in favor of !tagged_iterator #31321

Merged
merged 1 commit into from Jun 9, 2019

Conversation

Projects
None yet
6 participants
@jschaedl
Copy link
Contributor

commented Apr 29, 2019

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

  • fix tests

@jschaedl jschaedl changed the title [DI] deprecates tag tagged in favor of tagged_iterator [DI] deprecates tag !tagged in favor of !tagged_iterator Apr 29, 2019

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from d56bc8b to fad0d11 Apr 30, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 30, 2019

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from b5bbb98 to 6cd7c47 Apr 30, 2019

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch 3 times, most recently from 44c3096 to 6ebb340 Apr 30, 2019

@ro0NL

ro0NL approved these changes May 3, 2019

Show resolved Hide resolved UPGRADE-4.4.md Outdated
Show resolved Hide resolved src/Symfony/Component/DependencyInjection/CHANGELOG.md Outdated
Show resolved Hide resolved UPGRADE-5.0.md Outdated
Show resolved Hide resolved UPGRADE-5.0.md Outdated

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from 5398832 to 77bf619 May 5, 2019

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from 9fcf69d to 9d7f3be May 6, 2019

@nicolas-grekas
Copy link
Member

left a comment

(with one minor comment)

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch 3 times, most recently from 756015a to 14a94a3 May 6, 2019

@jschaedl

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2019

The TravisCi failure seems strange to me. I added the new argument type tagged_iterator to the services-1.0.xsd (See: https://github.com/symfony/symfony/pull/31321/files#diff-0e535437d820c04db5c34dcc4210e3da) and the XmlParsingException shouldn't occur. Maybe a caching issue?

@@ -60,7 +60,7 @@

<!-- transports -->
<service id="messenger.transport_factory" class="Symfony\Component\Messenger\Transport\TransportFactory">
<argument type="tagged" tag="messenger.transport_factory" />
<argument type="tagged_iterator" tag="messenger.transport_factory" />

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas May 22, 2019

Member

this means FWB now needs DI v4.4
this is why tests fail: the composer.json needs an update

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented May 22, 2019

(rebase needed also)

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from 14a94a3 to 79099a0 Jun 1, 2019

@jschaedl jschaedl changed the base branch from master to 4.4 Jun 1, 2019

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from 79099a0 to 116da00 Jun 1, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

The failing job is checking out branch 4.4 and loading dependencies with your patch applied.
This highlights that FrameworkBundle 4.4 is currently not compatible with DI 5.0 in your PR.
That's what needs to be fixed.

@jschaedl jschaedl force-pushed the jschaedl:di-tagged_iterator branch from 00acdff to 5597bd7 Jun 3, 2019

@xabbuh

xabbuh approved these changes Jun 9, 2019

Copy link
Member

left a comment

maybe we should keep tests for the legacy behaviour though to make sure that we do not accidentally break it

@nicolas-grekas nicolas-grekas force-pushed the jschaedl:di-tagged_iterator branch from 9bc7b56 to ab8fb18 Jun 9, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 9, 2019

Thank you @jschaedl.

@nicolas-grekas nicolas-grekas merged commit ab8fb18 into symfony:4.4 Jun 9, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Jun 9, 2019

feature #31321 [DI] deprecates tag !tagged in favor of !tagged_iterat…
…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
@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 9, 2019

@jschaedl would you mind sending a PR to remove the deprecated codes on master?

@jschaedl

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

@nicolas-grekas Sure, I'm happy to do that :-)

nicolas-grekas added a commit that referenced this pull request Jun 9, 2019

minor #31967 [DI] remove deprecated tag !tagged in favor of !tagged_i…
…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

OskarStark added a commit to symfony/symfony-docs that referenced this pull request Jun 11, 2019

minor #11708 [DI] Add deprecation info for !tagged (jschaedl)
This PR was submitted for the master branch but it was squashed and merged into the 4.4 branch instead (closes #11708).

Discussion
----------

[DI] Add deprecation info for !tagged

Fixes #11706

Feature PR: symfony/symfony#31321

Commits
-------

65263e1 [DI] Add deprecation info for !tagged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.