Update zend-coding standard #47
Conversation
| * @param ConfigInterface $config The configuration to compile from | ||
| * @param DependencyResolverInterface $resolver The resolver to utilize | ||
| * @param string|null $namespace Namespace to use for generated class; defaults | ||
| * to Zend\Di\Generated. |
There was a problem hiding this comment.
@webimpress The autofixer for the alignment rule does some odd things here? What is this indentation based upon?
src/Injector.php
Outdated
| * The runtime definition is used when null is passed or the parameter is omitted. | ||
| * @param Resolver\DependencyResolverInterface|null $resolver A custom resolver instance to resolve dependencies. | ||
| * The default resolver is used when null is passed or the parameter is omitted | ||
| * @param ConfigInterface|null $config A custom configuration to utilize. An empty configuration is used |
There was a problem hiding this comment.
@webimpress another nitpick of the alignment rule's cs fixer. Now the max line length is exceeded.
| * @param string|null $namespace Namespace to use for generated class; defaults | ||
| * to Zend\Di\Generated. | ||
| * @param LoggerInterface|null $logger An optional logger instance to log failures | ||
| * and processed classes. |
There was a problem hiding this comment.
Hm... these indent of second line of the comment are weird. Is it automatically fixed?
@xtreamwayz I think we need to check it.
test/TestAsset/Parameters.php
Outdated
| @@ -23,7 +23,7 @@ public function typelessRequired($bar) | |||
| { | |||
| } | |||
|
|
|||
| public function typehintOptional(A $fooOpt = null) | |||
| public function typehintOptional(?A $fooOpt = null) | |||
There was a problem hiding this comment.
Is it not affecting tests? Usually we don't want to run cs checks on TestAssets
There was a problem hiding this comment.
No it shouldn't since this just explicitly states the already implied nullabillity of this param. Should tests in general be excluded or just the test assets?
There was a problem hiding this comment.
TestAssets please
<exclude-pattern>*/TestAsset/*</exclude-pattern>
|
@webimpress I switched to dev-develop. Now the sniff fails. I guess due to active development: |
|
@tux-rampage Yes, I removed that sniff from my library, because there is one in PHP_CodeSniffer. The change is in my PR zendframework/zend-coding-standard#10 You can try to use that branch, as it contains the latest changes with more sniffs and then let us know if it is better for you. It contains fix for the comments in PHPDocs :) |
|
@tux-rampage would you like to try the latest alpha of Zend Coding Standard? |
Sure, but most likely not before next week. I'm quite busy at the moment. |
|
Closing now as we do not have yet ready zend-coding-standard and not sure if it will be ready before moving to Laminas. |
This PR aims to update the zend-coding standard to version 2
Currently this just updates the dependencies and applies autofix to see the outcome.
This is a pending issue: Update Zend Coding Standard to v2 #39
TODO
@alphafrom dependency's version constraintwebimpress/conting-standarddependency