search

middlewares / utils

Common utils used by PSR-15 middlewares
MIT License
51 stars 12 forks source link

composer: allow php ^8.0 #23

Closed solcik closed 3 years ago

solcik commented 3 years ago

closes https://github.com/middlewares/utils/issues/22

Should I send also migration from Travis to GitHub actions? Since Travis is ultra-slow these days and obviously does not have PHP 8.0 ready, GitHub actions are far better nowadays.

solcik commented 3 years ago

@oscarotero this one is also ready

You can see it here: https://github.com/solcik/utils/runs/1467297889

It is also prepared for https://coveralls.io/

solcik commented 3 years ago

@oscarotero @shadowhand @ecolinet @eusonlito

I am ready here for now. I am using these middlewares mentioned in this PR. I hope you are going to like my work done here. I will be glad for any feedback given. I might find more time - to do the others also.

franzliedke commented 3 years ago

Why does PHP 7.2 support need to be dropped in order to support PHP 8?

solcik commented 3 years ago

It is up to maintainers of course, but from my point of view 7.2 is in EOL. Current versions were used a long time without any update, so they are stable and can be used with 7.2 still.

franzliedke commented 3 years ago

This is a low-level library, used by many other libraries and applications. This change would prohibit these users from either

Supporting PHP 8 is great and obviously necessary. But let's not couple that with other, unrelated changes, IMHO!?

oscarotero commented 3 years ago

@solcik Thanks for this huge contribution. I have some comments:

Removing support for 7.2 is a breaking change, and it requires to be released as a major version. I don't want to do this for now (and I don't see any reason to remove 7.2), so I think we should maintain 7.2.

I also see a lot of stuff in the GA workflow (qa, test, coverage...) I don't now if we need this amount of tooling, I'd like to keep things simple. Anyway, it's a great contribution, I can modify it and reuse for the other middlewares packages.

There are other unrelated changes like replacing $this-> with self:: in the test cases that I don't know what for.

solcik commented 3 years ago

@oscarotero it is up to you, if you want, I am going to revert it to 7.2. (But it is not a breaking change, since composer would not install it as soon as somebody is on 7.2)

https://www.php.net/supported-versions.php

I think that tooling (phpstan, phpunit, coverage) is simple enough and is there for contributions - best UX. These are dev dependencies.

Upgraded phpunit - these methods are static. Therefore self instead of this.

oscarotero commented 3 years ago

Ok, thanks for your great contribution. I'll recover the 7.2 support and release a new version.