Conversation
mnapoli
left a comment
mnapoli
left a comment
There was a problem hiding this comment.
Thanks, could you also add a line in the changelog?
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ |
There was a problem hiding this comment.
The inheritdoc are not necessary as they are the default, I've been removing them in the codebase since they only make the code more complex. Could you remove them?
| * @author Matthieu Napoli <[email protected]> | ||
| */ | ||
| class DecoratorDefinition extends FactoryDefinition implements Definition, HasSubDefinition | ||
| class DecoratorDefinition extends FactoryDefinition implements HasSubDefinition |
There was a problem hiding this comment.
I like the fact that interfaces implemented are explicitly mentioned (else you have to navigate through plenty of files to get to the top interface). Could you revert that?
There was a problem hiding this comment.
I tend to go the other way around, phpstorm's EA extended plugin warns me if I'm missing interface methods
|
|
||
| /** | ||
| * @param object $instance | ||
| * @param ObjectDefinition $objectDefinition |
There was a problem hiding this comment.
This is redundant with the code, this doesn't need to be documented.
| public function __toString() | ||
| { | ||
| return 'Instance'; | ||
| } |
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} |
There was a problem hiding this comment.
This one should stay because the documentation is really inherited :)
|
I'l try to make the changes and add that line to changelog ASAP |
|
Thanks! |
2 participants
Covers #478 and #480