oauth2-bundle
oauth2-bundle copied to clipboard
Replace ext timecop with Carbon
Change to prepare support for php 8 #248
I had add a delta for token expiration checking because this is not really the purpose of integration/acceptance test to check that but the purpose of unit test. This is "mandatory" because league/oauth2-server use DateTimeImmutable object instead of a clock pattern.
I think the delta is the best way to handle that because in production case it can be something that can append so not really a point.
@X-Coder264 @HypeMC ?
@Orkin I'd prefer Carbon over Chronos.
- Carbon is a lot better maintained (it gets one or two releases per month vs Chronos which gets a couple of releases per year)
- Carbon is a much more popular choice as the PHP datetime library (169 717 086 downloads vs 16 699 937 downloads for Chronos according to Packagist, this is an order of magnitude difference). This means that it's much more likely that projects already require Carbon (as opposed to Chronos) meaning there's a higher chance that we won't be adding a "new" dependency for the users of our bundle.
- Carbon has much better CI testing (they test all supported PHP minor versions, but also all PHP patch versions where breaking changes were made to the Datetime PHP API under the hood).
- Carbon has native Doctrine DBAL type support for Symfony (https://github.com/briannesbitt/Carbon/tree/2.46.0/src/Carbon/Doctrine) which is one of the reasons I always use it over Chronos in my Symfony projects (and it's again one more reason why Symfony users are more likely to use it in their projects over Chronos too).
- I'm using Carbon in all my Symfony and Laravel applications so I'm generally more comfortable with that choice even though I see that Chronos has basically the same API.
@Orkin @HypeMC Thoughts?
Ok @X-Coder264 I made the change from Chronos to Carbon :)
@X-Coder264 @HypeMC update done 😄
@Orkin How did the entire disable access token saving feature end up on this branch? :smile:
Also some tests are missing the call to CarbonImmutable::setTestNow(null); in the teardown method.
@X-Coder264 don't worry I will revert, it's just a fail 😅, I used the web interface to resolve conflict for merging thoses features in develop to avoid waiting merge of this PR, bad idea
@X-Coder264 @HypeMC any news about this PR ?
@X-Coder264 @HypeMC a plan to merge this ?