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

Fix inconsistent switch case syntax #264

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

theodorejb
Copy link
Contributor

This must have been a typo, since it's the only instance where the non-standard syntax is used.

This must have been a typo, since it's the only instance where the non-standard syntax is used.
@WyriHaximus WyriHaximus added this to the v3.3.0 milestone Nov 15, 2024
@WyriHaximus
Copy link
Member

Hey, thanks @theodorejb for taking the time to file this PR. There are two things that bother me with this:

  • We have 10+ pairs of eyes on this and none of them spotted this across two PR, how did you find it? Did you ran into some kind of issue?
  • Why didn't our tests catch this 😅

@theodorejb
Copy link
Contributor Author

@WyriHaximus I ran a script across the top Composer packages to find any that were using the alternate switch case syntax, since I'm considering writing an RFC to deprecate it. In the top 1000 packages, this was one of only two where I found it used. All usages appeared to be typos (randomly mixed in with case statements using the standard syntax).

PHP-CS-Fixer will automatically find/fix these issues when using the @PER-CS ruleset (or even the old @PSR2 ruleset).

@WyriHaximus
Copy link
Member

WyriHaximus commented Nov 15, 2024

@theodorejb Ahh cool thanks, and tbh been doing PHP for 20+ years and only learned today this is valid syntax 🤦 . And honestly that RFC makes sense.

@clue clue added maintenance and removed bug labels Nov 19, 2024
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

@theodorejb TIL! Thanks for looking into this and best of luck with that RFC 👍

(I've marked this as maintenance to emphasize this shouldn't otherwise affect how this code functions.)

@clue clue merged commit 5f80055 into reactphp:3.x Nov 19, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants