Skip to content

Conversation

@lstrojny
Copy link
Owner

@lstrojny lstrojny commented Mar 6, 2021

Support for PHP 8 based on @phanan’s PR #236

Todos:

  • Upgrade PHPUnit
  • Update tests to work with PHPUnit's method changes, removals, and deprecations
  • Upgrade PHP-CS-Fixer
    • Temporarily allow PHP-CS-Fixer to fail for PHP~8, as the tool doesn't support it yet
  • Change the AbstractTestCase::expectArgumentError method to adapt to the new message format thrown in PHP 8 (actually, create a new method altogether).
  • Fix the few broken tests, which might come from broken functionalities themselves. @lstrojny maybe can lend a hand here?
  • Opt out for named parameters
  • Figure out backwards compatibility

use ReflectionFunction;

use function Functional\group;
use function get_defined_functions;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we or do we not import the root namespaced functions? It looks like a mix at the moment.

Copy link
Owner Author

@lstrojny lstrojny Mar 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s a mess, yeah. Functional shows it's age here (I just realized when updating the copyright notice it’s 10 years anniversary). When it started, use function … was not a thing. But yes, it should be imported everywhere.

Edit: I misread the question.

To the actual question: no, we backslash-ref import them. php-cs-fixer does it consistently for us. Still want to add php-cs-fixer to CI.

lstrojny and others added 4 commits March 7, 2021 00:49
@lstrojny
Copy link
Owner Author

lstrojny commented Mar 7, 2021

Most of these attributes can be removed, as their values are the same as the default.

Good point, done

@lstrojny lstrojny merged commit df44e36 into main Mar 7, 2021
@lstrojny lstrojny deleted the php8 branch March 7, 2021 00:08
This was referenced Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants