symfony

symfony copied to clipboard

Q	A
Branch?	6.1
Bug fix?	yes/no
New feature?	no
Deprecations?	no
Tickets
License	MIT
Doc PR

This PR relates to PR #44640 and shows all the changes I would like to make to improve code readability and fix a couple of minor issues. It is split into 3 commits to (hopefuly) make it easier to follow the changes.

Simplify the code when only installing

• Introduces a createLink function to enable an early return when only installing and any uneeded overhead.
• Adds a missing 2> NUL param in the build code.
• Replaces a proc_open with passthru in the build code to maintain consistency.

Ensure empty config values return any default

• Fixes $getEnvVar so that it returns the default for empty string values (72ce010c0be520f worked around this just for SYMFONY_PHPUNIT_VERSION).

Move code into relevant conditionals

• Moves all build code to if ($buildRequired) { ... }
• Changes the composer info call to composer show which now returns json data to avoid parsing the output.
• Moves and simplifies proc_open command code into if ($components) { ... }.

One downside to all this is that it will complicate any merge-ups from lower version fixes.

Hey!

I think @stevegrunwell has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

One downside to all this is that it will complicate any merge-ups from lower version fixes.

True. I've procrastinated reviewing this PR because changing for the sake of it is usually based on personal opinions and leads to time-consuming reviews... If you're still interested in pursuing this, can you please rebase?

True. I've procrastinated reviewing this PR because changing for the sake of it is usually based on personal opinions and leads to time-consuming reviews...

Yup, I'm sure I'm guilty of this too. However I've tried to keep the changes minimal in order to make the code more consistent and readable for the future.

Entirely up to you.

@nicolas-grekas How do you want to proceed here?

@fabpot I am prepared to do more work here if required (since things have changed slightly). If it is not worth the effort, then feel free to bin it.

Let me close here. Especially with PHPUnit 10 around, we need to reconsider our approach to the bridge anyway. Thanks for the PR.