8000 [DependencyInjection] ActionBundle integration: introduce _instanceof by dunglas · Pull Request #21357 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] ActionBundle integration: introduce _instanceof #21357

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

Closed
wants to merge 5 commits into from
Closed

[DependencyInjection] ActionBundle integration: introduce _instanceof #21357

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a48b9e
[DependencyInjection] ActionBundle integration: introduce _instanceof
dunglas Jan 19, 2017
9b9dbea
Factorize the ID generation logic
dunglas Jan 24, 2017
14e85c6
Fix CS
dunglas Jan 24, 2017
4e0b1e5
Clone the DOM when setting the parent
dunglas Jan 24, 2017
ec9c5b2
Make the service starting by a number
dunglas Jan 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Clone the DOM when setting the parent
  • Loading branch information
@dunglas
dunglas committed Jan 24, 2017
commit 4e0b1e549096a1a9215fbfe94f1fc4d64124b43b
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
}

if ($parentId) {
$s = clone $s;
$s->setAttribute('parent', $parentId);
Copy link
Contributor
@GuilhemN GuilhemN Jan 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

shouldn't it throw an exception if there is already a parent?

Copy link
Member

Choose a reason for hiding this comment

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

There is an issue here. This is altering the DOM node, which means it will impact the next services too (which will break things if they are not ChildDefinition themselves)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

}
$parentId = $this->generateInstanceofDefinitionId($id, $type, $file);
Expand Down
0