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

[Step 2 Upgrade] Bump requirement to php 8.0 #125

Merged
merged 19 commits into from Jan 12, 2024
Merged

[Step 2 Upgrade] Bump requirement to php 8.0 #125

merged 19 commits into from Jan 12, 2024

Conversation

samsonasik
Copy link
Contributor

@lisachenko here the 2nd step 👍 , bump to php 8.0

@lisachenko
Copy link
Member

lisachenko commented Jan 9, 2024 •
edited

@samsonasik There might be good parts which can be cherry-picked from the #122 or #121 - both of them were not merged.

@samsonasik
Copy link
Contributor Author

I will check 👍

samsonasik
samsonasik commented Jan 10, 2024
View reviewed changes
src/ReflectionType.php Outdated Show resolved Hide resolved
@samsonasik
Copy link
Contributor Author

samsonasik commented Jan 11, 2024 •
edited

The current skipped testGetModifiers() seems needs separate PR:

https://github.com/samsonasik/parser-reflection/blob/f220c210ca2fc443b71466c864325d9ad0bc0470/tests/ReflectionClassTest.php#L31-L35

@lisachenko Ready to merge 👍

@lisachenko
Copy link
Member

The current skipped testGetModifiers() seems needs separate PR:

https://github.com/samsonasik/parser-reflection/blob/f220c210ca2fc443b71466c864325d9ad0bc0470/tests/ReflectionClassTest.php#L31-L35

@lisachenko Ready to merge 👍

Awesome work! 👍 Let me check this today in the evening )

@lisachenko
Copy link
Member

Question here: do you plant to add PHP8 Attributes as separate independent PR?

lisachenko
lisachenko reviewed Jan 11, 2024
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@samsonasik
Copy link
Contributor Author

If you mean ReturnTypeWillChange attribute, that seems on php 8.1, the PR is step by step which this target is to make phpunit green first.

Otherwise, I may need failing test case for that to be implemented.

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

lisachenko
lisachenko approved these changes Jan 12, 2024
View reviewed changes
Copy link
Member

@lisachenko lisachenko left a comment

Choose a reason for hiding this comment

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

Approved, thanks

@lisachenko lisachenko merged commit d080526 into goaop:master Jan 12, 2024
2 checks passed
@samsonasik samsonasik deleted the bump-php80 branch January 12, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lisachenko lisachenko lisachenko approved these changes

Assignees
No one assigned
Labels
PHP8.0
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@samsonasik @lisachenko @scrutinizer-notifier