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

Allow newer thecodingmachine/safe #1649

Merged
merged 2 commits into from
Feb 3, 2022
Merged

Allow newer thecodingmachine/safe #1649

merged 2 commits into from
Feb 3, 2022

Conversation

mmoll
Copy link
Contributor

@mmoll mmoll commented Jan 18, 2022

thecodingmachine/safe#322 was needed for the phpstan check to pass.

@ricardoboss
Copy link

Are all these changes necessary? I mean wouldnt #1651 suffice?

@mmoll
Copy link
Contributor Author

mmoll commented Jan 18, 2022

@ricardoboss If your minimal update does the trick, I'm in favour of it. However, I had some phpstan warnings to fix locally (might be just my machine, though).

@ricardoboss
Copy link

I noticed you requested to merge into the base branch, whereas my request is for the PHP 8 feature branch (it wouldn't be an immediate update, but it would update safe once infection migrates to PHP 8).

@mmoll mmoll marked this pull request as ready for review January 19, 2022 14:39
@mmoll
Copy link
Contributor Author

mmoll commented Feb 1, 2022

should be ready now 💚

Copy link
Member

@sanmai sanmai left a comment

Choose a reason for hiding this comment

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

LGTM

src/TestFramework/CommandLineBuilder.php Outdated Show resolved Hide resolved
devTools/phpstan-src.neon Outdated Show resolved Hide resolved
@sanmai
Copy link
Member

sanmai commented Feb 3, 2022

@mmoll I'd appreciate if you don't force-push. Makes reviewing new changes a lot harder 😢

Copy link
Member

@sanmai sanmai left a comment

Choose a reason for hiding this comment

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

Still good.

@maks-rafalko Wanna take the helm?

@maks-rafalko
Copy link
Member

Thank you @mmoll & @sanmai !

@maks-rafalko maks-rafalko merged commit 4a1a2fd into infection:master Feb 3, 2022
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.

None yet

4 participants