Bump to PHP 7 #47
Bump to PHP 7 #47
Conversation
|
Tests pass, fail is related to Scrutinzer. I'd recommend moving to Coveralls and local coding standard checkers, since Scrutinizer is doing too much and failing with it quite often. |
|
I agree with you that we should investigate moving to an other code style checker. I think we should split up service usage. So I would like to start using Coverall. For code style I'm still looking for a service that works well. Problem is that most services are equal to Scrutinizer. But we should consider a reconfiguration. |
Awesome! I've added config and badge for coverage, just register repository there: https://coveralls.io/ - and disable "comments" and save, otherwise it will spam every PR and we will hate it :)
I've been using Scrutinizer for years, but it didn't actually help much. I've seen many errors and fails I had to debug with support. Since then I try to use prefferable CI I tools I know how they work, are open-source and can fix my code locally = no extra work. Thus, I moved from Scrutinizer to PHPStan and EasyCodingStandard (as in ReflectionDocBlock PR). It's all I need and it handles most of important codes checks that could possibly exist. Even Cyclomatic Complexity that you know from phpmd. Both easy to setup and easy to maintain. EasyCodingStandard even working for you with |
|
Scrutinizer probably also needs to be removed from settings of this repository: https://github.com/phpDocumentor/TypeResolver/settings/installations |
|
could you rebase this PR please. I'm missing the overview since I just merged #46 which had some overlap with this PR. |
|
I have added this and other reflection repos to coversall. |
|
Sure, I'll get to that tomorrow |
|
Rebased ✅ |
|
Scrutinizer needs to be disabled in settings of this repository, or it will keep pinging every PR. |
src/Types/ContextFactory.php
Outdated
| @@ -47,7 +47,7 @@ public function createFromReflector(\Reflector $reflector) | |||
| $fileName = $reflector->getFileName(); | |||
| $namespace = $reflector->getNamespaceName(); | |||
|
|
|||
| if (file_exists($fileName)) { | |||
| if ($fileName && file_exists($fileName)) { | |||
There was a problem hiding this comment.
could you make this more explicit, I generally don't trust the php casting model to booleans.
$fileName !== false
I think it would be nice to have a test for this as well.
There was a problem hiding this comment.
Well more correct would be is_strict(), since file_exists(true) fails as well.
I think test is not neccesary, it's php internal logic.
| @@ -98,7 +98,7 @@ public function testReadsAliasesFromProvidedNamespaceAndContent() | |||
| */ | |||
| public function testTraitUseIsNotDetectedAsNamespaceUse() | |||
| { | |||
| $php = '<?php | |||
| $php = '<?php declare(strict_types=1); | |||
There was a problem hiding this comment.
Think this change was unintended?
There was a problem hiding this comment.
Intended, this is consistent (tested) with the rest of the code.
| @@ -120,7 +120,7 @@ class FooClass { | |||
| */ | |||
| public function testAllOpeningBracesAreCheckedWhenSearchingForEndOfClass() | |||
| { | |||
| $php = '<?php | |||
| $php = '<?php declare(strict_types=1); | |||
There was a problem hiding this comment.
Think this change was unintended?
Done |
|
Done |
|
Awesome! Thanks |
Why?
What do you think?