Update build process to use stages and PHPCS #817
Conversation
| @@ -2,11 +2,11 @@ | |||
|
|
|||
| namespace Doctrine\Tests\Common\Reflection; | |||
|
|
|||
| use PHPUnit_Framework_TestCase; | |||
| use PHPUnit\Framework\TestCase; | |||
There was a problem hiding this comment.
Looks useless since you're using FQCN.
| @@ -23,7 +23,7 @@ | |||
| use Doctrine\Common\Proxy\Proxy; | |||
| use Doctrine\Common\Proxy\Exception\UnexpectedValueException; | |||
| use Doctrine\Common\Persistence\Mapping\ClassMetadata; | |||
| use PHPUnit_Framework_TestCase; | |||
| use PHPUnit\Framework\TestCase; | |||
There was a problem hiding this comment.
Looks useless since you're using FQCN.
| @@ -52,7 +52,7 @@ public function getClassNamespace($className) | |||
| { | |||
| $namespace = ''; | |||
| if (strpos($className, '\\') !== false) { | |||
| $namespace = strrev(substr( strrev($className), strpos(strrev($className), '\\')+1 )); | |||
| $namespace = strrev(substr(strrev($className), strpos(strrev($className), '\\')+1)); | |||
There was a problem hiding this comment.
Shouldn't there be spaces around +?
There was a problem hiding this comment.
It should indeed. We need to find the sniff and add it to doctrine/coding-standard then. Could you do it?
There was a problem hiding this comment.
There is Squiz.WhiteSpace.OperatorSpacing, but it wasn't compatible with declare(strict_types=1) and reports it as violation. Ended up with a hack to ignore declare in our private CS. :/
|
|
||
| const foo = \stdClass::class; | ||
| const FOO = \stdClass::class; |
There was a problem hiding this comment.
Missing visibility, maybe Doctrine CS should enforce it for 7.1+?
There was a problem hiding this comment.
That sounds like an awesome addition, since we have PHP 7.1+ on our packages I think we should enforce people to make things explicit. We should find/build the sniff to do that though.
There was a problem hiding this comment.
Have a look at slevomat/coding-standard, they provide a bunch of very interesting 7.1+ sniffs; I'm happily using them elsewhere. 👍
There was a problem hiding this comment.
Seems neat, could you send a PR to doctrine/coding-standard?
| @@ -80,37 +78,36 @@ function (Proxy $proxy, $method, $params) use (&$counter) { | |||
| $proxy->__setInitializer(null); | |||
|
|
|||
| $proxy->publicField = 'modifiedPublicField'; | |||
| $counter += 1; | |||
| $counter += 1; | |||
There was a problem hiding this comment.
Just wondering whether this alignment is correct? = are aligned, but + is not, it looks strange.
There was a problem hiding this comment.
AFAIK that's our standard. Maybe we can discuss that on doctrine/coding-standard
|
The PHPStan error on Travis is strange (job 844.3), why is it looking for generated files? :/ |
@Majkl578 no clue at all, it works just fine if you create the directory, quite weird. |
Making the test runner a bit more strict.
Modifying things to use Travis-CI build stages.
lcobucci
force-pushed
the
update-build-process
branch
from
6d372dd to
29d9f1b
Compare
|
@Majkl578 you were too fast HAHAHA I was about to add that commit 😜 |
|
|
|
@Majkl578 feel free to merge it 😉 |
|
Very well. |
|
I'll let you know when the changes to doctrine/coding-standard are ready. 👍 |
|
@Majkl578 awesome, thanks! |
Applying the same things we have on the other projects