Conversation
morozov
left a comment
There was a problem hiding this comment.
Please retarget against 2.12.x.
e9034c5 to
732c66c
Compare
28900a6 to
c07b200
Compare
composer.json
Outdated
| "psalm/plugin-phpunit": "^0.12.0", | ||
| "symfony/console": "^2.0.5|^3.0|^4.0|^5.0", | ||
| "vimeo/psalm": "^3.17.2" | ||
| "vimeo/psalm": "^3.18.2" |
There was a problem hiding this comment.
@morozov should we drop the ^ part here? that way we avoid discrepancies between local runs and CI (at the cost of a warning from Composer I think)
There was a problem hiding this comment.
The ^ doesn’t block later versions so why a warning?
There was a problem hiding this comment.
I checked and you are right, there is no warning. Maybe that changed, or I have faulty memory. That said, I was proposing to remove the ^, that's when I thought you would get a warning, you won't get a warning with it that's for sure.
There was a problem hiding this comment.
Maybe that changed, or I have faulty memory.
No, Composer would definitely trigger a warning in some cases if some dependency constraint was declared as a specific version.
should we drop the
^part here?
Why now? The specific version is locked in the lock file which we still commit to Git. I believe it should be done separately from this specific upgrade.
There was a problem hiding this comment.
I wasn't require we do it now, but in general. Now would be as good a time as any.
There was a problem hiding this comment.
Now would be as good a time as any.
The way how we manage development dependencies is for the most part initiated by the work being done in #4386 which seems stuck. Once we decide to make a change, we'll make it for all dependencies at once.
This patch comes from an external contributor and just updates a dependency. I don't see much point in increasing the scope.
1ccd983 to
2ee4fae
Compare
|
Thanks, @simPod. |
|
Thanks for patience with reviews :) |
Summary
In order to fix issues related but not explicitly caused by #4403