Skip to content

Conversation

kocsismate
Copy link
Member

No description provided.

&reftitle.returnvalues;
<para>
An ICU error code indicating either success, failure or a warning.
An ICU error code indicating either success, failure or a warning. Returns &false; on failure.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
An ICU error code indicating either success, failure or a warning. Returns &false; on failure.
An ICU error code indicating either success, failure or a warning. Returns &false; when there is no error to fetch.

Or something similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, yes, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it true actually?

/* Fetch the object (without resetting its last error code ). */
co = Z_INTL_CALENDAR_P(object);
if (co == NULL)
    RETURN_FALSE;

So it returns false when the Calendar_object cannot be retrieved, doesn't it?

Copy link
Member

Choose a reason for hiding this comment

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

Right, well I would have expected this to throw an Error but oh well. (actually maybe we need to check what the macro does)

Copy link
Member Author

Choose a reason for hiding this comment

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

Checked it :) It returns the struct

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

There is still one remaining comment from the previous review which does need to be addressed but other than the couple of other nits it looks alright.

@kocsismate
Copy link
Member Author

Has the last commit addressed your comments, right? Can I merge it?

@kocsismate kocsismate merged commit 65a1dc5 into php:master Oct 11, 2021
@kocsismate kocsismate deleted the stub-intl-calendar branch October 11, 2021 13:02
marcovtwout pushed a commit to marcovtwout/doc-en that referenced this pull request Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants