-
Couldn't load subscription status.
- Fork 8k
ext/intl: dateformatter settimezone changes on success, returning true #10790
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
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.
Returning true makes sense to me. Could we add this to the tests that call ut_datefmt_set_timezone_id?
|
This is technically an API break, but the current API is so whack that I think it's fine... |
3565411 to
97e6f3d
Compare
sure |
d8ffc93 to
f6a8163
Compare
f6a8163 to
84b446d
Compare
UPGRADING
Outdated
| . Changed DOMCharacterData::appendData() tentative return type to true. | ||
|
|
||
| - Intl: | ||
| . IntlDateformatter::setTimeZone (and its alias dafmt_set_timezone) |
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.
FYI: it's actually the other way around. dafmt_set_timezone has the implementation, and IntlDateformatter::setTimeZone is the alias. @alias foo is stubs basically mean that "this function/method is an alias of foo"
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.
good point
84b446d to
f3fe321
Compare
UPGRADING
Outdated
| . Changed DOMCharacterData::appendData() tentative return type to true. | ||
|
|
||
| - Intl: | ||
| . datefmT_set_timezone (and its alias IntlDateformatter::setTimeZone) |
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.
| . datefmT_set_timezone (and its alias IntlDateformatter::setTimeZone) | |
| . datefmt_set_timezone (and its alias IntlDateformatter::setTimeZone) |
f3fe321 to
fc8b8a8
Compare
| RETURN_THROWS(); | ||
| } | ||
|
|
||
| RETURN_TRUE; |
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.
Looks like an accidental unrelated change?
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.
yes..
fc8b8a8 to
5b05cb8
Compare
|
@iluuu1994 does it look ok now ? 🙂 |
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.
Looks good from my side, thank you @devnexen 🙂
like setcalendar.