-
Notifications
You must be signed in to change notification settings - Fork 12
relax dev dependencies #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@mikespub thanks! Will check it tomorrow 👍 |
Thanks. No changes were needed to support the tests with phpunit 10, 11 & 12 here, unlike middlewares/cache which needed a few updates |
|
@mikespub Hi, thanks for your input. Let me share my thoughts though on why it's v10. I don't see any problem supporting v10 only as it exactly matches our PHP constraint: >=8.1 and the last update was 1 week ago. My idea was to use the phpunit version that does exactly that, and then when the time comes to drop 8.1 we upgrade phpunit to v11 (>=8.2) which would again match our version. So, I don't see any benefit on doing what you did. I feel like it's anticipating what will go next (specially on the other package that requires changes to code). Also, I would feel different about it if the change would be for production which would block (in our case) the phpunit version of the project using this lib. I think it adds complexity to have 3 versions of the same lib. Imagine that the |
|
Hi @filisko I understand your point of view, but I thought we might as well be a bit pro-active in checking & supporting newer versions of dependencies. The last times we went through this, it was to support psr/http-message 2.x and PHP 8.4. Now I wanted to check the new major releases of phpunit for all supported PHP versions, and as it turns out both middlewares/cache and its required mikespub/micheh-psr7-cache had minor compatibility issues. And since I updated this package for psr/http-message 2.x too, I wanted to do the same thing here, and it turns out that the tests are fine for all versions. So we might as well allow them now, so that none of us need to come back in 6 months or a year again :-) Note: I'm perfectly fine if we keep v10 for PHP 8.1, but we also support PHP 8.2, 8.3 and 8.4, and phpunit/phpunit has new major releases for each PHP version. Relaxing the dev dependency like I did allows us to test this package with the phpunit version most appropriate for each PHP version, while keeping compatibility for all versions. |
|
I agree that perhaps in this case it's ok but as I mentioned earlier, TestCase could have 3 different implementations. I upgraded all production required packages of the libs of this org. to their latest. Perhaps only 1 was missing and I have it spotted. What where the compatibility issues? Why package's version was not changed? We will have to come anyways to drop 8.1, 8.2, 8.3 .. :) As I said, it's anticipating work, it doesn't improve anything (proved by the tests) so now we have 2 more versions Will check the other package with the changes |
|
Hi @filisko as there are no other changes here, feel free to close this PR if you prefer. The package version is based on tags, so I'll leave that decision to the maintainer :-) |
|
@mikespub thank you though, I'm willing to see you more around in our other middlewares too! :) I'm the main maintainer now, |
Support phpunit 11+