Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

use an opaque type for values yielded by TimeZoneNameIter #221

Closed
BurntSushi opened this issue Jan 25, 2025 · 0 comments
Closed

use an opaque type for values yielded by TimeZoneNameIter #221

BurntSushi opened this issue Jan 25, 2025 · 0 comments
Labels
breaking change Issues that require a breaking change for resolution.

Comments

@BurntSushi
Copy link
Owner

Specifically, this iterator: https://docs.rs/jiff/latest/jiff/tz/struct.TimeZoneNameIter.html

Currently, it always yields a String. But this is somewhat restrictive and requires alloc. I think we should also parameterize it over a lifetime borrowed from the originating TimeZoneDatabase. And then add an as_str<'a>(&'a self) -> &'a str method (specifically with a lifetime attached to the value yielded and not the TimeZoneDatabase, so as to permit Cow shenanigans or an owned representation internally). I think this is perhaps overly restrictive, but it is maximally flexible from an API evolution perspective.

I will probably keep the API alloc-only for jiff 0.2, so there won't be any immediate improvement here. But it at least gives us a chance to evolve it in semver compatible ways should the need arise.

@BurntSushi BurntSushi added the breaking change Issues that require a breaking change for resolution. label Jan 25, 2025
BurntSushi added a commit that referenced this issue Jan 25, 2025
This should be more friendly to future API evolution here. I previously
did the simplest thing possible and wasn't really thinking about
core-only environments.

We still don't support any tzdb in core only environments, but we
theoretically could in the future. (Although there are a number of
issues to figure out. This merely fixes one of them.)

Fixes #221
BurntSushi added a commit that referenced this issue Jan 26, 2025
This should be more friendly to future API evolution here. I previously
did the simplest thing possible and wasn't really thinking about
core-only environments.

We still don't support any tzdb in core only environments, but we
theoretically could in the future. (Although there are a number of
issues to figure out. This merely fixes one of them.)

Fixes #221
BurntSushi added a commit that referenced this issue Jan 27, 2025
This should be more friendly to future API evolution here. I previously
did the simplest thing possible and wasn't really thinking about
core-only environments.

We still don't support any tzdb in core only environments, but we
theoretically could in the future. (Although there are a number of
issues to figure out. This merely fixes one of them.)

Fixes #221
BurntSushi added a commit that referenced this issue Jan 30, 2025
This should be more friendly to future API evolution here. I previously
did the simplest thing possible and wasn't really thinking about
core-only environments.

We still don't support any tzdb in core only environments, but we
theoretically could in the future. (Although there are a number of
issues to figure out. This merely fixes one of them.)

Fixes #221
BurntSushi added a commit that referenced this issue Feb 1, 2025
This should be more friendly to future API evolution here. I previously
did the simplest thing possible and wasn't really thinking about
core-only environments.

We still don't support any tzdb in core only environments, but we
theoretically could in the future. (Although there are a number of
issues to figure out. This merely fixes one of them.)

Fixes #221
BurntSushi added a commit that referenced this issue Feb 2, 2025
This should be more friendly to future API evolution here. I previously
did the simplest thing possible and wasn't really thinking about
core-only environments.

We still don't support any tzdb in core only environments, but we
theoretically could in the future. (Although there are a number of
issues to figure out. This merely fixes one of them.)

Fixes #221
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
breaking change Issues that require a breaking change for resolution.
Projects
None yet
Development

No branches or pull requests

1 participant