Skip to content

ext/intl: GH-20255 IntlDateFormatter adding proleptic gregorian calen… - #21101

Closed
devnexen wants to merge 2 commits into
php:masterfrom
devnexen:gh20255
Closed

ext/intl: GH-20255 IntlDateFormatter adding proleptic gregorian calen…#21101
devnexen wants to merge 2 commits into
php:masterfrom
devnexen:gh20255

Conversation

@devnexen

Copy link
Copy Markdown
Member

…dar support.

To be consistent with DateImmutable class, we add the possibility to set the calendar in a (real) proleptic gregorian via a new flag constant. For now, intention needs to be clear but can be made default eventually.

…lendar support.

To be consistent with DateImmutable class, we add the possibility to set
the calendar in a (real) proleptic gregorian via a new flag constant.
For now, intention needs to be clear but can be made default eventually.
@devnexen
devnexen marked this pull request as ready for review January 31, 2026 16:44
@devnexen
devnexen requested a review from kocsismate as a code owner January 31, 2026 16:44
@devnexen
devnexen requested a review from Girgias February 7, 2026 23:25
@devnexen

devnexen commented Apr 4, 2026

Copy link
Copy Markdown
Member Author

ping :)

@Girgias
Girgias requested a review from derickr April 6, 2026 12:46
@Girgias

Girgias commented Apr 6, 2026

Copy link
Copy Markdown
Member

I don't know anything about calendars or dates really. Asking Derick to review this instead.

@Girgias
Girgias removed their request for review April 6, 2026 12:47

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@devnexen devnexen closed this in 25a2d97 Aug 3, 2026
@kocsismate

Copy link
Copy Markdown
Member

@devnexen Somehow, this commit fails the build for the benchmark suite, see https://github.com/php/php-src/actions/runs/30867397842/job/91862130279#step:16:13190 . I used "bisecting" locally, and I ended up at this commit that first failed compilation (actually as far as I can see, the configure script fails).

Unfortunately, I don't see based on the diff what could have gone wrong, nor I found anything useful in the logs. Do you have any idea how what may be the problem?

@LamentXU123

LamentXU123 commented Aug 5, 2026

Copy link
Copy Markdown
Member

I don't see any thing that could go wrong in this PR. I doubt there are other reasons.

@devnexen

devnexen commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

The first thing coming up to my head is missing header maybe. e.g. <limits> (the C++ one)

@devnexen

devnexen commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

@LamentXU123 feel free to make a PR. Cheers !

LamentXU123 added a commit that referenced this pull request Aug 5, 2026
This fixes #21101 (comment)

Some extreme build environments happen to get std::numeric_limits through
transitive includes. So we need to put a Include manually

Co-authored-by: David Carlier <devnexen@gmail.com>
@kocsismate

Copy link
Copy Markdown
Member

@LamentXU123 @devnexen Thanks for the fix, it worked! :)

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.

5 participants