Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Feature-gated jiff support #130

Closed
LeoniePhiline opened this issue Oct 30, 2024 · 6 comments
Closed

Feature-gated jiff support #130

LeoniePhiline opened this issue Oct 30, 2024 · 6 comments

Comments

@LeoniePhiline
Copy link
Contributor

I am in the process of migrating code from chrono to the superior jiff library.

The Schedule iterator currently only yields chrono::DateTime<Z>.

With the rise of a new date and time handling library which does some things better than chrono, it might be useful to make chrono optional (feature-gated) and add jiff support (also behind a feature gate).

Are you at all open to having jiff support added to the crate?

#87 may be related, as jiff handles timezones better than chrono.

@LeoniePhiline
Copy link
Contributor Author

LeoniePhiline commented Oct 30, 2024

A fork already exists, which completely replaces chrono by jiff, thus diverging from upstream: https://github.com/maxcountryman/jiff-cron (diff)

Might it be more efficient to join forces? (cc @maxcountryman)

@maxcountryman
Copy link

@LeoniePhiline a feature sounds like a lot of work. That's why I dropped support for chrono altogether in my fork. I plan to continue to maintain that fork as underway uses it for scheduling.

@LeoniePhiline
Copy link
Contributor Author

LeoniePhiline commented Oct 31, 2024

@maxcountryman I tend to agree.

What are the plans for your fork? There has been increased activity in https://github.com/zslayton/cron recently. (Recent merges)

Are you going to rebase onto upstream regularly, port changes, or let the fork fully diverge?

Would you mind opening your fork's Issues section to keep track of synchronization of features and fixes?

Edit: I performed a rebase of the latest upstream master onto your jiff migrated fork: jiff-cron#3

@zslayton
Copy link
Owner

Are you at all open to having jiff support added to the crate?

Yeah, I think that would be wise. jiff seems like it's where things are heading.

It would be nice to have a migration/shim story for the users using the old chrono-based APIs, even if that's just a rudimentary feature-gated .to_chrono() extension method on the jiff datetime iterator we return.

@LeoniePhiline
Copy link
Contributor Author

LeoniePhiline commented Oct 31, 2024

@zslayton

It would be nice to have a migration/shim story for the users using the old chrono-based APIs, even if that's just a rudimentary feature-gated .to_chrono() extension method on the jiff datetime iterator we return.

I like your idea of using jiff internally and providing a chrono shim.

This sounds like much less long-term effort than making the entire implementation generic over a DateTime provider or littering the code with #[cfg(feature = "...")] attributes.

For jiff-cron#3 I have rebased the fork onto the latest changes to your master branch.

Does seeing this migrated and rebased codebase inspire you to any kind of low-maintenance chrono compat shim implementation approach? Does the orphan rule allow us to come up with any kind of elegant shim as long as jiff and chrono do not provide conversion functionality themselves?

I would very much prefer to re-unify the fork with upstream as soon as backwards compatibility is implemented.

A possible contra argument: library users working in a chrono setting might not want to (transitively) depend on and compile jiff - even if it is used purely as cron implementation detail. Is there value in a purely chrono-based cron library?

@maxcountryman
Copy link

One thing I think should be considered here: jiff is not widely adopted yet and there's some indication it might take quite a while before it is (SQLx for example is not planning to support it immediately). What this means in practice is chrono isn't going anywhere and I very much suspect some folks will want or even need a chrono version of this crate now and for the foreseeable future.

All this to say, it might actually make more sense to have separate implementations. This would also allow an implementation of say jiff to refactor around jiff specifically (for instance, timezone-aware arithmetic is directly supported whereas chrono and chrono-tz are not capable of this in the same way).

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

No branches or pull requests

3 participants