Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[DependencyInjection] Allow array attributes for service tags#47364

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:6.2fromaschempp:feature/tag-array-attributes
Oct 24, 2022

Conversation

@aschempp
Copy link
Contributor

QA
Branch?6.2
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#38339,#38540
LicenseMIT
Doc PRsymfony/symfony-docs#...

This adds array attribute support for container service tags as described in#38339. The most notable change is the additions to the XML schema for services, because that's the current limitation for not using array.

This is reopening#38540 as new PR since the other one was ("accidentially") closed 😊
/cc@nicolas-grekas

ruudk reacted with heart emoji
@carsonbotcarsonbot added this to the6.2 milestoneAug 23, 2022
@aschemppaschempp changed the titleFeature/tag array attributes[DependencyInjection] Allow array attributes for service tagsAug 23, 2022
@carsonbot
Copy link

Hey!

I think@nusje2000 has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks for reopening.
LGTM, just some nitpicking.

@ruudk
Copy link
Contributor

@aschempp Just wanted to say how happy I am that you didn't give up on this idea and finished the PR 👏 🙌 💙

aschempp and roboparker reacted with heart emoji

@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 19, 2022
@nicolas-grekasnicolas-grekasforce-pushed thefeature/tag-array-attributes branch from8be8031 toedd8d77CompareOctober 24, 2022 09:40
@fabpot
Copy link
Member

Thank you@aschempp.

aschempp reacted with hooray emoji

@ju1ius
Copy link
Contributor

ju1ius commentedJan 11, 2023•
edited
Loading

Hi@aschempp, I can't get this feature to work.
The following config:

$configurator->services()  ->set(Foo::class)  ->tag('foo', ['bar' => ['baz' =>42]);

throws:

[Symfony\Component\DependencyInjection\Exception\RuntimeException]at symfony/dependency-injection/Compiler/CheckDefinitionValidityPass.php line 67:A "tags" attribute must be of a scalar-type for service "App\Foo", tag "foo", attribute "bar".

The tag is accepted by theContainerConfigurator, but rejected by theCheckDefinitionValidityPass...

Am I missing something or should I open an issue ?

@nicolas-grekas
Copy link
Member

@ju1ius looks like you're the first to actually use the feature 😅
Can you please send us a fix with a test case?

@ju1ius
Copy link
Contributor

@nicolas-grekas OK, will take a stab at it during the week.

@ju1ius
Copy link
Contributor

@nicolas-grekas Done in#48958 😉

nicolas-grekas added a commit that referenced this pull requestJan 13, 2023
…bute values (ju1ius)This PR was squashed before being merged into the 6.2 branch.Discussion----------[DependencyInjection] fixes validation of non-scalar attribute values| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#48956| License       | MIT| Doc PR        | NoneAs a follow-up to#47364, this PR updates the `CheckDefinitionValidityPass` to allow (possibly nested) arrays of scalars in service tags attribute values.Please see#48956 for context.Commits-------88b3e15 [DependencyInjection] fixes validation of non-scalar attribute values
nicolas-grekas added a commit that referenced this pull requestMar 14, 2024
…so a 'name' property (lyrixx)This PR was merged into the 6.4 branch.Discussion----------[DependencyInjection] fix XmlDumper when a tag contains also a 'name' property| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        || License       | MITSince#47364, the tag can contain an **array** of attributes.And since#47364, SF uses it to store workflow metadata and also a **name** property.It likely broke the XML generation, and so it brokes `debug:router` command for instance.Sorry, I didn't notice it before!Before the patch, I got that in my container var/cache/dev/App_KernelDevDebugContainer.xml```xml<tag>workflow<attribute name="name">article</attribute><attribute name="metadata"><attribute name="title">Manage article</attribute></attribute></tag>```After, I got```xml<tag name="workflow">  <attribute name="name">article</attribute>  <attribute name="metadata">    <attribute name="title">Manage article</attribute>  </attribute></tag>```Commits-------4a4b011 [DependencyInjection] fix XmlDumper when a tag contains also a 'name' property
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@welcoMatticwelcoMatticwelcoMattic approved these changes

Assignees

No one assigned

Labels

DependencyInjectionFeature❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

Support for non-scalar tag attributes

8 participants

@aschempp@carsonbot@ruudk@fabpot@ju1ius@nicolas-grekas@welcoMattic@xabbuh
[8]ページ先頭
©2009-2025 Movatter.jp