#20921 [Config] Provide shorthand methods for ArrayNodeDefinition::pr… #20923
Conversation
skafandri
commented
Dec 14, 2016
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #20921 |
| License | MIT |
…ion::prototype()
| @@ -80,6 +80,76 @@ public function prototype($type) | |||
| } | |||
|
|
|||
| /** | |||
| * Shorthand for prototype('variable'). | |||
There was a problem hiding this comment.
could be removed, IMO it doesn't add any additional value
There was a problem hiding this comment.
I think so, I think that the method name is self explanatory. But wouldn't break the code standards check?
There was a problem hiding this comment.
Quite the contrary :) We are trying to only add phpdoc when it adds value.
| * | ||
| * @return ScalarNodeDefinition | ||
| */ | ||
| public function prototypeScalar() |
There was a problem hiding this comment.
What about using scalarPrototype instead (same for the other new methods)?
There was a problem hiding this comment.
scalarPrototype sounds better for me. The only motivation behind the current naming is to group the methods in an IDE auto complete list.
There was a problem hiding this comment.
I understand that. But at least PhpStorm is even with the other names able to suggest all of these methods when typing prototype (if I am not completely mistaken).
There was a problem hiding this comment.
Yes you're right. That's not a big issue. Most IDE autocomplete are clever enough to show methods containing rather than starting with a string.
There was a problem hiding this comment.
I agree, my IDE does that too, I was only thinking about some friends who are still using IDEs with left prefix only autocomplete support (Netbeans 6). If you think we shouldn't care about this legacy, I am into it.
|
I don't understand the generated diff of fabbot.io Is that hook open source? :) |
|
👍 |
1 similar comment
|
👍 |
| @@ -227,6 +227,48 @@ public function testNormalizeKeys() | |||
| $this->assertFalse($this->getField($node, 'normalizeKeys')); | |||
| } | |||
|
|
|||
| public function testPrototypeVariable() | |||
|
A single test with multiple asserts to cover all the cases. I am fine with that. What do you think guys? |
|
Thank you @skafandri. |
…eDefinition::pr… (skafandri) This PR was squashed before being merged into the 3.3-dev branch (closes #20923). Discussion ---------- #20921 [Config] Provide shorthand methods for ArrayNodeDefinition::pr… | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20921 | License | MIT Commits ------- 22b8490 #20921 [Config] Provide shorthand methods for ArrayNodeDefinition::pr…
|
👍 |
This PR was merged into the 2.7 branch. Discussion ---------- [Config] Improve PHPdoc / IDE autocomplete | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes-ish | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> The missing pieces for fixing the autocomplete in IDE's (tested in `PhpStorm 2016.3.2`). Each method seems to be found now for the framework bundle configuration 😱 Actually uses #20923 where needed, but i think we should go with it consistently. Relates to #20290 (comment) Commits ------- 3f3c6e0 [Config] Improve PHPdoc / IDE autocomplete