Skip to content

PHP 7.3: Use break instead of continue within a switch case#10037

Merged
mdawaffe merged 1 commit intomasterfrom
kraftbj-patch-1
Aug 23, 2018
Merged

PHP 7.3: Use break instead of continue within a switch case#10037
mdawaffe merged 1 commit intomasterfrom
kraftbj-patch-1

Conversation

@kraftbj
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj commented Aug 20, 2018

Starting in PHP 7.3 (currently in Beta 2), PHP will throw a warning when using continue within a switch to confirm intent. In PHP, within switch, break and continue do the same thing while in other languages continue would behave like PHP's continue 2.

This would preserve the same behavior and avoid the warning. See https://github.com/php/php-src/blob/php-7.3.0beta2/UPGRADING

Based on the current timeline, this would need to land no later than our December release (6.8 based on our monthly clip). PHP 7.3 is expected to go into GA on December 13th. Since this is a backwards compatible change, no reason to wait though.

Testing instructions:

  1. Before patch, run something via PHP 7.3. See warning.
  2. Apply patch, repeat, see no error.

Proposed changelog entry for your changes:

  • Improves compatibility with the upcoming PHP 7.3

Starting in PHP 7.3 (currently in Beta 2), PHP will throw a warning when using `continue` within a `switch` to confirm intent. In PHP, within `switch`, `break` and `continue` do the same thing while in other languages `continue` would behave like PHP's `continue 2`.

This would preserve the same behavior and avoid the warning. See https://github.com/php/php-src/blob/php-7.3.0beta2/UPGRADING
@kraftbj kraftbj added Bug When a feature is broken and / or not performing as intended General [Status] Needs Review This PR is ready for review. labels Aug 20, 2018
@kraftbj kraftbj requested a review from a team as a code owner August 20, 2018 16:22
@jetpackbot
Copy link
Copy Markdown
Collaborator

That's a great PR description, thank you so much for your effort!

Generated by 🚫 dangerJS

Copy link
Copy Markdown
Member

@mdawaffe mdawaffe left a comment

Choose a reason for hiding this comment

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

Nice! Thanks :)

@kraftbj kraftbj added this to the 6.5 milestone Aug 20, 2018
@abidhahmed abidhahmed added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 21, 2018
@mdawaffe mdawaffe merged commit cbeae8f into master Aug 23, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 23, 2018
@mdawaffe mdawaffe deleted the kraftbj-patch-1 branch August 23, 2018 21:25
zinigor added a commit that referenced this pull request Aug 31, 2018
* Synchronized the media summary class.

* Reverted back to break because #10037, props @mdawaffe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended General

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants