When year is in some far future - use offsets of some similar year before 2038 #881
Conversation
| @@ -169,6 +169,11 @@ | |||
| return i; | |||
| } | |||
| } | |||
|
|
|||
| var alternative = this._getAlternative(timestamp); | |||
There was a problem hiding this comment.
I don't think it makes sense for this logic to be in _index, since it's not returning the index for an until that represents the timestamp. It would have the correct offset. I think it would be better to create a different method that wraps this information instead, and have the other method call the existing _index.
| @@ -330,6 +350,16 @@ | |||
| } | |||
| } | |||
|
|
|||
| function findYearLike(year) { | |||
| for (var j = 2038; j > 2000; j--) { | |||
There was a problem hiding this comment.
If we aren't worried about strange exceptions (like when a timezone skips a day entirely), we use the fact that there's a 28-year cycle in which the calendar repeats itself, instead of performing a loop.
There was a problem hiding this comment.
Yes, I already fixed that, will push tomorrow, it was beta version to check if that logics (of searching similar years) will work at all
| @@ -330,6 +350,16 @@ | |||
| } | |||
| } | |||
|
|
|||
| function findYearLike(year) { | |||
| for (var j = 2038; j > 2000; j--) { | |||
| if (new Date(year + '-01-01').getDay() === new Date(j + '-12-31').getDay() && | |||
There was a problem hiding this comment.
You're repeating the same check here instead of comparing the first day of the week of the target year with the last day of the week of the candidate year, instead of comparing the first and last days of the week of the target year with those of the candidate year.
There was a problem hiding this comment.
fixed that already, will push changes tomorrow
There was a problem hiding this comment.
Any news about the fix @ellenaua ? :)
|
@ellenaua any news about this PR? :) |
|
Hi @ellenaua, May I ask if there is any update on this PR? Thanks a lot by the way! |
|
Please have a look at #921 |
|
The problem I raised in #768 is not fixed by this PR, or by #921. See #825 for a related issue in the other direction (very old timestamps). The core problem is that the current build process is highly dependent on the version of Ideally there'd be a standardised CI setup for building new data files (using GitHub Actions or similar), which could better guarantee data quality. I could take a look into doing this, but to be honest, I don't see many contributor PRs being merged on this repo, so I'm not sure it would be accepted. |
Ah, @gilmoreorless already troubleshooted this. Nice. Closing this for now. About using CI to release for a more stable result -- we could, as long as it's very transparent. Or we could have a small docker file in the repo and use that for zic/zdump. |
This is related to bugs:
#798
#768