Skip to content

Conversation

derickr
Copy link
Member

@derickr derickr commented Jun 9, 2022

Ideally this should be done by reimporting timelib, but I haven't made a release of that yet, so we'll do it with a "hot fix".

@derickr derickr requested a review from Girgias June 9, 2022 15:58
@derickr derickr self-assigned this Jun 9, 2022
@cmb69
Copy link
Member

cmb69 commented Jun 9, 2022

Do you have a reference on the weekdays of the dates in the tests? Respective tests with ext/calendar (since there is no year 0 there, I decreased the years by one):

var_dump(
    jddayofweek(gregoriantojd(1, 1, -1501), CAL_DOW_SHORT),
    jddayofweek(gregoriantojd(1, 1, -2147483649), CAL_DOW_SHORT),
    jddayofweek(gregoriantojd(1, 27, -292277022658), CAL_DOW_SHORT),
);

gives:

string(3) "Fri"
string(3) "Tue"
string(3) "Mon"

@derickr
Copy link
Member Author

derickr commented Jun 10, 2022

@cmb69, you were right about jddayofweek(gregoriantojd(1, 1, -1501), CAL_DOW_SHORT), having to return Friday. There was indeed another bug in my code with multiples of 100 (negative).

The other two can't be checked with ext/calendar, as its algorithm states:

 * VALID RANGE
 *
 *     4714 B.C. to at least 10000 A.D.

@derickr
Copy link
Member Author

derickr commented Jun 10, 2022

And FWIW, the results now match with what https://www.fourmilab.ch/documents/calendar/ does.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

I think we're good to go. :)

@derickr derickr merged commit fe97a5a into php:PHP-8.0 Jun 17, 2022
@derickr derickr deleted the bug77342 branch June 17, 2022 08:50
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