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

fix: Support named args in ParallelConfigFactory::detect() #8026

Merged
merged 6 commits into from
May 20, 2024
Merged

fix: Support named args in ParallelConfigFactory::detect() #8026

merged 6 commits into from
May 20, 2024

Conversation

Wirone
Copy link
Member

@Wirone Wirone commented May 20, 2024 •
edited

Fixes #8025

@Wirone Wirone added the topic/parallel Issues related to parallel runner label May 20, 2024
@Wirone Wirone self-assigned this May 20, 2024
@coveralls
Copy link

coveralls commented May 20, 2024 •
edited

Coverage Status

coverage: 95.648%. remained the same
when pulling ca5ba5d on Wirone:codito/parallel-config-named-args
into cd1922b on PHP-CS-Fixer:master.

@Wirone Wirone force-pushed the codito/parallel-config-named-args branch from 829f985 to 8ace150 Compare May 20, 2024 11:39
@Wirone Wirone force-pushed the codito/parallel-config-named-args branch from 8ace150 to f114cfe Compare May 20, 2024 12:00
Wirone and others added 5 commits May 20, 2024 22:01
@Wirone
Add failing test
fdc3d4c
@Wirone
Support named args in ParallelConfigFactory::detect()
addb6b0
@Wirone
Simplify support for named args
2a04c6d 
We can't filter args for PHP <8 because then it's impossible to use default value for `$filesPerProcess`, calling `ParallelConfigFactory::detect(null, 360)` will just use 360 files per process and default timeout (120). Since `ParallelConfig`'s constructor requires explicit values, we can't keep `null` values passed to factory, because it would lead to type error, so we need to re-use default values on factory level (hence changed visibility of the constants).
@Wirone @keradus
Make consts internal
29ffc71 
Co-authored-by: Dariusz Rumiński <dariusz.ruminski@gmail.com>
@Wirone
Use constants ase values for tests' asserts
1235431
@Wirone Wirone force-pushed the codito/parallel-config-named-args branch from 3aaa1c2 to 1235431 Compare May 20, 2024 20:02
@Wirone Wirone enabled auto-merge (squash) May 20, 2024 20:10
@Wirone Wirone merged commit 4526271 into PHP-CS-Fixer:master May 20, 2024
27 checks passed
@Wirone Wirone deleted the codito/parallel-config-named-args branch May 20, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Steveb-p Steveb-p Steveb-p left review comments

@keradus keradus keradus approved these changes

@julienfalque julienfalque Awaiting requested review from julienfalque

@kubawerlos kubawerlos Awaiting requested review from kubawerlos

Assignees

@Wirone Wirone

Labels
topic/parallel Issues related to parallel runner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ParallelConfigFactory using named parameters behaves unexpectedly when omiting first argument
4 participants
@Wirone @coveralls @keradus @Steveb-p