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

PHP 8.1: Implemented new in initializers #819

Merged
merged 2 commits into from
Oct 22, 2021
Merged

PHP 8.1: Implemented new in initializers #819

merged 2 commits into from
Oct 22, 2021

Conversation

kukulich
Copy link
Collaborator

No description provided.

@kukulich kukulich mentioned this pull request Oct 13, 2021
Closed
11 tasks
@kukulich kukulich force-pushed the php81-new-in-initializers branch 2 times, most recently from 5febac9 to 08c3d83 Compare October 14, 2021 16:25
@kukulich kukulich marked this pull request as ready for review October 14, 2021 16:44
Ocramius
Ocramius requested changes Oct 14, 2021
View reviewed changes
src/NodeCompiler/CompileNodeToValue.php Outdated Show resolved Hide resolved
@kukulich kukulich marked this pull request as draft October 22, 2021 08:59
@kukulich kukulich force-pushed the php81-new-in-initializers branch from 08c3d83 to 58f9ff2 Compare October 22, 2021 14:24
@Ocramius Ocramius added this to the 5.0.0 milestone Oct 22, 2021
@kukulich
Copy link
Collaborator Author

Ok, can I lower the MSI again? It looks it's already failing in master...

@Ocramius
Copy link
Member

@kukulich yes please - I was trying to see which mutants were introducing the problem, but nothing related to this specific patch.

@Ocramius
Copy link
Member

To me, it seems like coverage is ignoring assert() calls, perhaps we have assertions disabled (by accident) in .ini 🤷

Ocramius
Ocramius approved these changes Oct 22, 2021
View reviewed changes
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 👍

@kukulich
Copy link
Collaborator Author

I think the problem is that we use assert to make PHPStan/Psalm happy but there are places where the assert cannot never fail.

@Ocramius
Copy link
Member

@kukulich those should in theory be picked up by roave/infection-static-analysis-plugin, but since they are uncovered, the plugin never kicks in to squish those mutants.

@kukulich kukulich marked this pull request as ready for review October 22, 2021 15:00
@Ocramius
Copy link
Member

I see you're trying to improve this in #825 - meanwhile, merging here :)

@Ocramius Ocramius self-assigned this Oct 22, 2021
@Ocramius Ocramius merged commit 57d1217 into Roave:5.0.x Oct 22, 2021
@Ocramius Ocramius changed the title PHP 8.1: Implemented "new" in initializers PHP 8.1: Implemented new in initializers Oct 22, 2021
@kukulich kukulich deleted the php81-new-in-initializers branch October 24, 2021 15:18
@Ocramius Ocramius mentioned this pull request Oct 29, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asgrim asgrim asgrim left review comments

@ondrejmirtes ondrejmirtes ondrejmirtes left review comments

@Ocramius Ocramius Ocramius approved these changes

Assignees

@Ocramius Ocramius

Labels
enhancement reflection compatibility
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kukulich @Ocramius @ondrejmirtes @asgrim