Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: update phpunit to version 9 #932

Merged
merged 7 commits into from May 5, 2022
Merged

feat: update phpunit to version 9 #932

merged 7 commits into from May 5, 2022

Conversation

Chris53897
Copy link
Contributor

@Chris53897 Chris53897 commented Apr 26, 2022 •
edited

As discussed in #928 this is the first task needed for modernization (in my point of view).

  • drop support of php < 8
  • update phpunit 5/8 => 9 (with migration of phpunit.xml.dist)

There is of course a lot of space for improvments. But i do not want to do, to many things at once.

@pierredup @johnkrovitch
Maybe you can branch from master a new develop-branch 2.x for modernization.
So i can take new branch as base for this PR. Advantage: master stays clean for other changes.

From that point we can improve that branch by ...

[ ] Add Typehints on internal API (avoid BC breaks)
[ ] Use new Language Features of PHP 8
[ ] Remove external deprecations (PhpUnit, ...)
[ ] Move Travis to github actions (Run tests on each repo separately)
[ ] Update Dependencies: Example: Possible BC Break: php-http/guzzle6-adapter => php-http/guzzle7-adapter
[ ] TODO

And after all that is done, new Interfaces and more complex code changes can be done.

So this single PR ist not getting bigger and bigger.

WDYT?

@Chris53897
Copy link
Contributor Author

Tests are failing because of deprecation warnings for PhpUnit 10.
GitHub Actions need to be adjusted for that.

@Chris53897 Chris53897 mentioned this pull request Apr 29, 2022
Closed
@pierredup pierredup added this to the next milestone May 4, 2022
@pierredup pierredup linked an issue May 4, 2022 that may be closed by this pull request
Payum v2 with php 8.1 as minimum requirements #928
Open
14 tasks
@pierredup pierredup force-pushed the feature/drop-support-php-7 branch from 15d0c6e to a71432c Compare May 4, 2022 13:13
@pierredup pierredup changed the title feat: drop support of php < 8, update phpunit 5/8 => 9 feat: update phpunit to version 9 May 4, 2022
@pierredup
Copy link
Member

@Chris53897 Thanks for working on this! I agree this PR was just getting too big. I have made some smaller changes on the master branch (updating version to 2.x, removing PHP 7 from composer, removing travis config file (since travis hasn't been used for a while) etc).

I have also reverted the commit that added all the types, so that we can focus only on the PHPUnit update in this PR.

Other changes can then be made in separate smaller PRs targeting the master branch. For things like adding type-hints, I also think it should be done incrementally and not all in one go (E.G take a few interfaces, add the types, and do the next few in another PR), just to make it easy to review and ensure we can revisit the types.
I want to try and avoid using the mixed type anywhere. If there is currently a method that accepts mixed, we should re-visit it to determine what type do we want, and if necessary, create the new interfaces or classes if we want to define a new type.

@Chris53897
Copy link
Contributor Author

@pierredup thanks for having a look. I am already working on an PR in the meantime for typehints. I will try to get tests working and will respond here after that is done.

@pierredup pierredup mentioned this pull request May 5, 2022
Merged
@pierredup pierredup changed the base branch from master to 1.x May 5, 2022 12:45
@pierredup pierredup force-pushed the feature/drop-support-php-7 branch 3 times, most recently from 56e4886 to d2b6a45 Compare May 5, 2022 12:59
@pierredup pierredup force-pushed the feature/drop-support-php-7 branch from d2b6a45 to da053ac Compare May 5, 2022 13:04
@pierredup pierredup force-pushed the feature/drop-support-php-7 branch from aff7162 to a7c2208 Compare May 5, 2022 14:48
@pierredup pierredup merged commit 1e88814 into Payum:1.x May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierredup pierredup pierredup approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
Next Major (v2.0.0)
Status: Done
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.

Payum v2 with php 8.1 as minimum requirements
3 participants
@Chris53897 @pierredup @Chris8934