-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/intl: IntlDateFormatter class removing redundant error message info. #13465
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
Also correcting new IntlChar class constants typos.
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.
LGTM, notes for future changes.
} else { | ||
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR, | ||
"intltz_create_enumeration: invalid argument type", 0); | ||
"invalid argument type", 0); | ||
RETURN_FALSE; | ||
} |
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 feels like it should be a TypeError, something to remember as future improvements
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.
Actually this looks like it should just use Fast ZPP with a int|string type check.
if (!tzobj->initialized) { | ||
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR, | ||
"intltz_from_date_time_zone: DateTimeZone object is unconstructed", | ||
"DateTimeZone object is unconstructed", | ||
0); | ||
RETURN_NULL(); | ||
} |
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.
Note, this should probably an Error as uninitialized objects means something BAD happened.
/** @cvalue UCHAR_ID_COMPAT_MATH_START */ | ||
public const int PROPERTY_ID_COMPT_MATH_START = UNKNOWN; | ||
public const int PROPERTY_ID_COMPAT_MATH_START = UNKNOWN; | ||
/** @cvalue UCHAR_ID_COMPAT_MATH_CONTINUE */ | ||
public const int PROPERTY_ID_COMPT_MATH_CONTINUE = UNKNOWN; | ||
public const int PROPERTY_ID_COMPAT_MATH_CONTINUE = UNKNOWN; | ||
#endif |
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.
Are these new constants?
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
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 to merge then :D
Also correcting new IntlChar class constants typos.