JuliaTime / TZJData.jl

MIT License
1 stars 0 forks source link

Consider new major for each tzdata version #31

Open green-nsk opened 1 week ago

green-nsk commented 1 week ago

Recent TZJData release broke our code. tz"CET" now throws because CET timezone has been deprecated by IANA recently. Our CI process caught the issue, but there's no reasonable workaround because 1) we don't depend directly on TZJData, only via TimeZones.jl 2) this is only a minor release, so package manager doesn't think twice about pulling it.

Please consider making each release a major release s.t. semantic versioning works as expected.

TimeZones.jl should consider doing the same, though I guess it's less clear-cut and an issue for that package anyway.

omus commented 1 week ago

but there's no reasonable workaround

Available options without changing the semver setup:

  1. Use TimeZone("CET", TimeZones.Class(:ALL)) which lets you to continue to use legacy time zones
  2. Use the non-deprecated name for "CET" which is "Europe/Brussels". I need to make the replacement names more easily discoverable: https://github.com/JuliaTime/TimeZones.jl/issues/469 (Note: == will fail as they have different names. The transitions are equivalent though)
  3. Add the dependency on TZJData.jl and set your compat appropriately (e.g. TZJData = "~1.2")

Please consider making each release a major release s.t. semantic versioning works as expected.

Deprecations are commonly included in minor version bumps for Julia packages. The difference here is that TimeZones.jl doesn't allow the use of legacy/deprecated time zones by default. I could see an argument to be made for tying the default TimeZone.Class with the --depwarn flag and having --depwarn=error be the only mode where TimeZone.Class(:LEGACY) is disallowed.

green-nsk commented 1 week ago

Add the dependency on TZJData.jl and set your compat appropriately (e.g. TZJData = "~1.2")

Indeed that would help, made me re-read how ~compat works. Knowing to add a TZJData dependency just for semver is not obvious though, we don't directly depend on it. TBH I wasn't even aware that TImeZones and TZData are separate packages until yesterday.

We went with replacing CET for more specific timezone as you suggested, seems like we'd have to do this anyway at some point.

I could see an argument to be made for tying the default TimeZone.Class with the --depwarn flag and having --depwarn=error be the only mode where TimeZone.Class(:LEGACY) is disallowed.

I'm unfamiliar with the information in TZJData and what changes can be made between the releases. Could the next tzdata version entirely remove CET or some other time zones? If there's no certainty about this, I would err on the cautious side, that's why I suggested major versions for each new release.

If we know that timezones are never deleted completely, what you suggested sounds appropriate.

omus commented 1 week ago

I'm unfamiliar with the information in TZJData and what changes can be made between the releases. Could the next tzdata version entirely remove CET or some other time zones? If there's no certainty about this, I would err on the cautious side, that's why I suggested major versions for each new release.

If we know that timezones are never deleted completely, what you suggested sounds appropriate.

That's a good point. I think a good compromise in this situation would be to add a test which ensures that the new release includes all of the time zone names of the previous release. This would enforce that each new release is a supserset of the old release. I agree if these deprecated time zones were removed we should make a breaking TZJData.jl release.

One aspect where this gets tricky is that TimeZones.jl wouldn't really care that time zones were dropped so users would still need a TZJData.jl compat entry for this to be effective against breakage.