Skip to content

Conversation

@arsalanyavari
Copy link

@arsalanyavari arsalanyavari commented May 2, 2025

fix the #11, #33 and #36 issues

@fzerorubigd
Copy link
Collaborator

The leap check function has two parts. The first is using the official data to simply get the leap status based on a table can be found here
If you want to change the leap year function, then you have to provide a test for all the years in the doc and the result should be based on the doc. the range out of that can't be tested, and we can ignore the test for them.

@arsalanyavari
Copy link
Author

I have added the file sources/test_kit/jalali/leap_range.c to the project and hardcoded the mentioned leap years in it. From now on, after the make, you can check the leap years using ./sources/test_kit/jalali/leap_range.

The logic of the new jalali_is_jleap() function has passed all these tests as well.

@fzerorubigd
Copy link
Collaborator

I tried several times to get to this one and merge it. but it changes everything in one single PR and every time I get to it, I decide to simply ignore it since it is too much.
while the title is to fix a flag, it changes everything. For that reason alone I am not comfortable merging this one.

@arsalanyavari
Copy link
Author

Thanks for your response.
This bug is also caused by an error in calculating the leap years. Therefore the pull request to fix it requires all the changes we have made.

Also, the commit messages are telling, and this pull request is made up of several commits.

Feel free to ask about any of our changes and we'll happily respond.

@fzerorubigd
Copy link
Collaborator

fzerorubigd commented Sep 26, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants