Skip to content

Fix #76999: mb-regex-set-options return current options - #5264

Closed
cmb69 wants to merge 1 commit into
php:PHP-7.3from
cmb69:cmb/76999
Closed

Fix #76999: mb-regex-set-options return current options#5264
cmb69 wants to merge 1 commit into
php:PHP-7.3from
cmb69:cmb/76999

Conversation

@cmb69

@cmb69 cmb69 commented Mar 13, 2020

Copy link
Copy Markdown
Member

When setting new options, mb_regex_set_options() is supposed to
return the previous options.

When setting new options, `mb_regex_set_options()` is supposed to
return the *previous* options.
@cmb69

cmb69 commented Mar 13, 2020

Copy link
Copy Markdown
Member Author

This is obviously a BC break, but now the function behaves as documented. Alternative: change documentation, but keep the function's behavior.

@Girgias

Girgias commented Mar 13, 2020

Copy link
Copy Markdown
Member

IMHO it's better to change the function as most of our setter function return the previous option IIRC

@cmb69 cmb69 added the Bug label Mar 15, 2020
@nikic

nikic commented Mar 16, 2020

Copy link
Copy Markdown
Member

I think we should do this, but not for 7.3. 7.4/master should be fine.

@cmb69

cmb69 commented Mar 16, 2020

Copy link
Copy Markdown
Member Author

7.4/master should be fine.

Makes sense. So whether this should go into 7.4 should be decided by the RMs. @derickr, @petk, what do you think?

@cmb69

cmb69 commented Mar 27, 2020

Copy link
Copy Markdown
Member Author

Okay, applied to master as 9e77d5a.

@cmb69 cmb69 closed this Mar 27, 2020
@cmb69
cmb69 deleted the cmb/76999 branch March 27, 2020 09:35
cmb69 added a commit to cmb69/doc-en that referenced this pull request Mar 27, 2020
php-pulls pushed a commit to php/doc-en that referenced this pull request Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants