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.

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

PHP 8.1: Installation and green build #767

Merged
merged 15 commits into from
Oct 14, 2021
Merged

PHP 8.1: Installation and green build #767

merged 15 commits into from
Oct 14, 2021

Conversation

kukulich
Copy link
Collaborator

@kukulich kukulich commented Sep 18, 2021

No description provided.

@kukulich kukulich marked this pull request as draft September 18, 2021 12:36
@kukulich kukulich force-pushed the php81 branch 2 times, most recently from bc1a962 to eab0e03 Compare September 18, 2021 17:49
composer.json Outdated Show resolved Hide resolved
@kukulich kukulich force-pushed the php81 branch 2 times, most recently from 537a230 to 60d5a83 Compare September 18, 2021 20:05
@kukulich
Copy link
Collaborator Author

Btw I think is good to merge the basic PHP 8.1 support because probably there may be new BC breaks so one major BR release can solve them all.

And some PHP 8.1 features can be supported right away (readonly properties) so we can add features one by one. And all new added features can be checked against both PHP versions.

@Ocramius
Copy link
Member

IMO we need to walk before we run. Would prefer to have mainline having all of 8.0 feature support, then 8.1 support

@Ocramius
Copy link
Member

We can hold off tagging until then

@WyriHaximus
Copy link
Contributor

We can hold off tagging until then

Or release 8.1 support as 5.1?

@Ocramius
Copy link
Member

Ocramius commented Sep 18, 2021

Depends if that will lead to further BC breaks - that's why perhaps holding off tagging is a better choice at first :)

@kukulich
Copy link
Collaborator Author

This may or may not be BC break: 4bee6ff

@kukulich kukulich force-pushed the php81 branch 5 times, most recently from 1f9c8f4 to d41ebbd Compare September 23, 2021 14:34
@kukulich
Copy link
Collaborator Author

@WyriHaximus You can try to implement intersection types here :) You should need new version of https://github.com/nikic/PHP-Parser/releases/tag/v4.13.0

@kukulich kukulich force-pushed the php81 branch 2 times, most recently from 0aa61f1 to 1d1ece4 Compare September 23, 2021 20:35
@kukulich kukulich changed the title PHP 8.1 start PHP 8.1 Sep 27, 2021
@kukulich kukulich force-pushed the php81 branch 2 times, most recently from e258f6d to a418ae0 Compare September 27, 2021 15:45
@kukulich kukulich mentioned this pull request Oct 3, 2021
13 tasks
@WyriHaximus
Copy link
Contributor

@kukulich You can pick intersection types up from WyriHaximus-labs@7c7af32

@kukulich
Copy link
Collaborator Author

kukulich commented Oct 4, 2021

@WyriHaximus Done 👍

@WyriHaximus
Copy link
Contributor

@kukulich Awesome! Any preference on what I pick up next?

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Welp, thought I sent in the review, but my uplink died :D

composer.json Show resolved Hide resolved
src/Reflection/Adapter/ReflectionClass.php Show resolved Hide resolved
src/Reflection/Adapter/ReflectionClass.php Show resolved Hide resolved
src/Reflection/Adapter/ReflectionClass.php Outdated Show resolved Hide resolved
src/Reflection/Adapter/ReflectionClassConstant.php Outdated Show resolved Hide resolved
src/Reflection/Adapter/ReflectionProperty.php Outdated Show resolved Hide resolved
.github/workflows/phpunit.yml Show resolved Hide resolved
src/Reflection/ReflectionType.php Outdated Show resolved Hide resolved
@kukulich
Copy link
Collaborator Author

@Ocramius Expected result... New asserts (because of 11ef581) lowered the mutation score and the build is red :(

@Ocramius
Copy link
Member

lowered the mutation

Half as bad

build is red :(

I think it is acceptable to lower the MT thresholds, since we are indeed expanding to cover 2 PHP versions.

If I understand this correctly, we may go back up once we drop PHP 8.0 in a far future (perhaps +6 months)

@Ocramius Ocramius added this to the 5.0.0 milestone Oct 14, 2021
@Ocramius Ocramius added enhancement dependencies Pull requests that update a dependency file labels Oct 14, 2021
@kukulich
Copy link
Collaborator Author

1d0914c and we are green again :)

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

Thanks for investigating this, @kukulich! Massive kudos for the upgrade work and investigation: I'm surprised we got it running on 8.0 and 8.1 after all!

@@ -32,7 +33,8 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
ini-values: memory_limit=-1
# No deprecated errors
Copy link
Member

Choose a reason for hiding this comment

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

Yay PHP :|

TBH, we should handle deprecations, but can't deal with them inside third-party tools, obviously.

@@ -6,6 +6,7 @@ or backwards compatibility (BC) breakages occur.
## 5.0.0

### BC Breaks
* All adapters are `final`
Copy link
Member

Choose a reason for hiding this comment

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

BTW, my hope is to generate a whole list of BC breaks through roave/backward-compatibility-check when we get close to tagging this 👍

@Ocramius Ocramius self-assigned this Oct 14, 2021
@Ocramius Ocramius merged commit 7bf7481 into Roave:5.0.x Oct 14, 2021
@kukulich kukulich deleted the php81 branch October 14, 2021 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants