Skip to content

[Config] Backport string|null api for node names #26297

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

Merged
merged 1 commit into from
Mar 19, 2018
Merged

[Config] Backport string|null api for node names #26297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/Symfony/Component/Config/Definition/BaseNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ abstract class BaseNode implements NodeInterface
protected $attributes = array();

/**
* @param string $name The name of the node
* @param NodeInterface $parent The parent of this node
* @param string|null $name The name of the node
* @param NodeInterface|null $parent The parent of this node
*
* @throws \InvalidArgumentException if the name contains a period
*/
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Config/Definition/Builder/NodeBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ public function end()
/**
* Creates a child node.
*
* @param string $name The name of the node
* @param string $type The type of the node
* @param string|null $name The name of the node
Copy link
Contributor

Choose a reason for hiding this comment

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

It does not make sense to change this when you don't change all the other places as well like https://github.com/ro0NL/symfony/blob/2d72b61613131160d2f4716f71bae7a55a9f4153/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php#L39 and https://github.com/ro0NL/symfony/blob/2d72b61613131160d2f4716f71bae7a55a9f4153/src/Symfony/Component/Config/Definition/Builder/NodeBuilder.php#L52

Copy link
Contributor

Choose a reason for hiding this comment

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

Also what does a node without a name do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok my bad.. it's clarified only on 4.0: https://github.com/symfony/symfony/blob/4.0/src/Symfony/Component/Config/Definition/BaseNode.php#L39

Should be backported to 2.7 as string|null AFAIK, this being the first instance :)

In general it's used to create a prototyped node: https://github.com/symfony/symfony/blob/4.0/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php#L77

I think it makes sense to keep string API when using a named factory: scalarNode('string') vs node(null, 'scalar'). Which seems to be the case already; https://github.com/symfony/symfony/blob/4.0/src/Symfony/Component/Config/Definition/Builder/NodeBuilder.php#L52

* @param string $type The type of the node
*
* @return NodeDefinition The child node
*
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ abstract class NodeDefinition implements NodeParentInterface
protected $attributes = array();

/**
* @param string $name The name of the node
* @param string|null $name The name of the node
* @param NodeParentInterface|null $parent The parent
*/
public function __construct($name, NodeParentInterface $parent = null)
Expand Down