-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/intl: breakiterator::setText returns false on failure. #10820
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
Conversation
@@ -164,7 +164,7 @@ public function preceding(int $offset): int {} | |||
public function previous(): int {} | |||
|
|||
/** @tentative-return-type */ | |||
public function setText(string $text): ?bool {} // TODO return false instead of null in case of failure | |||
public function setText(string $text): bool {} // TODO return false instead of null in case of failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove todo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
absolutely.
40d9be4
to
3987cea
Compare
3987cea
to
01d51d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM because the return value change is not fundamental so that at least already existing loose comparisons continue to work
@@ -78,7 +78,9 @@ PHP 8.3 UPGRADE NOTES | |||
|
|||
- Intl: | |||
. datefmt_set_timezone (and its alias IntlDateformatter::setTimeZone) | |||
now returns true on sucess, previously null was returned. | |||
now returns true on success, previously null was returned. | |||
. breakiterator::setText now returns false on failure, previously |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
. breakiterator::setText now returns false on failure, previously | |
. IntlBreakIterator::setText() now returns false on failure, previously |
Function brances (()
) in the upgrading file are sometimes omitted, sometimes not, but personally, I'd prefer to have them if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this one is IMHO fine because null to false doesn't change a lot of expectation
No description provided.