tisonkun / cronexpr

Calculate the next timestamp matching a given crontab pattern
https://docs.rs/cronexpr
Apache License 2.0
49 stars 2 forks source link

Leaky abstraction: `jiff` #18

Closed maxcountryman closed 1 month ago

maxcountryman commented 1 month ago

The documentation states:

The mainly difference is that this crate always requires the timezone to be specified in the crontab expression. This is because the timezone is necessary to determine the next timestamp.

I believe this introduces a leaky abstraction (i.e. with jiff), as it forces users to handle timezone details where they typically wouldn't need to in the standard crontab format. Ideally, the abstraction would hide this complexity, but in this case, the internal need for time zone information "leaks" through. This also creates a compatibility issue with the standard crontab format, which doesn't require timezones.

A possible improvement could be to infer UTC when no timezone is explicitly provided, preserving the simplicity users expect from crontab expressions while maintaining the necessary functionality.

BurntSushi commented 1 month ago

A possible improvement could be to infer UTC when no timezone is explicitly provided

I think TimeZone::system() would make a lot more sense. I would be very surprised if my cron system didn't interpret unadorned dates and times using my system's time zone.

tisonkun commented 1 month ago

Thanks for your reporting. I'll add a new method parse_crontab_with(options) to support infer timezone when it is missing.

tisonkun commented 1 month ago

infer UTC when no timezone is explicitly provided

my cron system didn't interpret unadorned dates and times using my system's time zone

So here we have to default flavors already. I'll make this fallback manner optional. PR will be ready for review now.