search

google / cctz

CCTZ is a C++ library for translating between absolute and civil times using the rules of a time zone.
Apache License 2.0
597 stars 166 forks source link

Update zoneinfo files to 2024b #298

Closed devbww closed 2 months ago

google-cla[bot] commented 2 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

derekmauro commented 2 months ago

Thanks! I started working on this on Friday as well.

There is a test failure in TimeZoneEdgeCase.WET. I looks like WET was made an alias for Europe/Lisbon, almost as an arbitrary choice of replacement. But that removes the edge cases tested by TimeZoneEdgeCase.WET. I haven't looked to see if that means we are losing test coverage for the non-existent transitions tested by TimeZoneEdgeCase.WET, and if we need to replace them by other non-existent transitions for coverage.

devbww commented 2 months ago

There is a test failure in TimeZoneEdgeCase.WET. I looks like WET was made an alias for Europe/Lisbon, almost as an arbitrary choice of replacement.

Sigh. I don't know why the upstream stuff has to be so unstable. Oh well.

But that removes the edge cases tested by TimeZoneEdgeCase.WET. I haven't looked to see if that means we are losing test coverage for the non-existent transitions tested by TimeZoneEdgeCase.WET, and if we need to replace them by other non-existent transitions for coverage.

I don't think we're losing any coverage, and that the test could well be removed. But to err on the side of caution, I'll propose a test tweak instead.

Unfortunately, I am currently not in the position to build/test anything, so I could easily get things wrong. I guess we'll see if it turns green. But also given that inability, please feel free to ignore this PR altogether and move forward with your stuff. I was only hoping to make the CCTZ size of things easier, but until I can build/test again I may well be making it worse. :-(

derekmauro commented 2 months ago

Unfortunately, I am currently not in the position to build/test anything, so I could easily get things wrong. I guess we'll see if it turns green. But also given that inability, please feel free to ignore this PR altogether and move forward with your stuff. I was only hoping to make the CCTZ size of things easier, but until I can build/test again I may well be making it worse. :-(

I got so tired of Kokoro constantly breaking, and with the possibility of losing such a valuable contributor, I added GitHub Actions in 6624659e01e73e41527d6b27036e9f79a556560f. If you want to rebase onto or merge 6624659e01e73e41527d6b27036e9f79a556560f, I think the tests should just work for you. I'll delete the Kokoro tests shortly.

devbww commented 2 months ago

I'll open a new PR.