Conversation
|
The action will be visible only after this is merged, but you can see how it looks at greg0ire#1 |
greg0ire
force-pushed
the
psalm-sa
branch
2 times, most recently
from
c122725 to
e14f55a
Compare
|
@localheinz @bendavies please review |
|
I force pushed to add |
|
LGTM
Marco Pivetta
http://twitter.com/Ocramius
http://ocramius.github.com/
…On Sat, May 2, 2020 at 1:42 PM Grégoire Paris ***@***.***> wrote:
I force pushed to add cs2pr
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#105 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABFVEFKYGR2W7V4HNU4GH3RPQBI3ANCNFSM4MXJ5RKQ>
.
|
| run: "composer install --no-interaction --no-progress --no-suggest" | ||
|
|
||
| - name: "Psalm" | ||
| run: "vendor/bin/psalm --output-format=checkstyle | cs2pr" |
There was a problem hiding this comment.
psalm has native support, no need for cs2pr
vendor/bin/psalm --show-info=false --stats --output-format=github --threads=4
| xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" | ||
| > | ||
| <projectFiles> | ||
| <directory name="lib/Doctrine/Persistence" /> |
There was a problem hiding this comment.
Woops, didn't mean to leave those out
There was a problem hiding this comment.
Actually, this will be hard to do until 2.0 and until we use phpunit 8, we should probably leave those out.
There was a problem hiding this comment.
I'm giving a try in case that's not that big of an issue.
There was a problem hiding this comment.
@bendavies I managed to make it work! Locally. And now the build Travis fails because phpunit 8 requires php 7.2. So I'm removing these commits, but they are still on a branch on my machine 🙈
If it were just me, I would drop 7.1, but I think most maintainers are not very keen on that on bugfix branches
| name: "Continuous Integration" | ||
|
|
||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
What would be the point of that?
There was a problem hiding this comment.
do you require PRs to be up to date before merging?
if not, then a passing PR could fail on the branch after merging?
There was a problem hiding this comment.
We don't, and I hadn't thought of that possibility. That being said, it will not prevent the PR from being merged.
greg0ire
force-pushed
the
psalm-sa
branch
2 times, most recently
from
58955a4 to
05eae23
Compare
| with: | ||
| path: "~/.composer/cache" | ||
| key: "${{ matrix.php-version }}-composer-locked-${{ hashFiles('composer.lock') }}" | ||
| restore-keys: "${{ matrix.php-version }}-composer-locked-" |
psalm.xml
Outdated
| <?xml version="1.0"?> | ||
| <psalm | ||
| totallyTyped="false" | ||
| errorLevel="5" |
There was a problem hiding this comment.
Lower is better, and we can have lvl 4 with 0 modifications, so let's do that.
If we added type hints to this project, the SymfonyFileLocator would not be compatible with the interface. Changing the phpdoc of the interface is a small enough BC-break that allows us to get back to a valid state.
7 participants
No description provided.