switch from chrono to time #1790
Conversation
src/cmd/serve.rs
Outdated
| let format = format_description!("[year]-[month]-[day] [hour]:[minute]:[second] UTC"); | ||
| println!( | ||
| "Change detected @ {}", | ||
| OffsetDateTime::now_utc().format(&format).unwrap().to_string() | ||
| ); |
There was a problem hiding this comment.
@Keats This timestamp is the only reason the root crate needs a dependency on time. It also includes very very minor regression because with chrono, Zola could print the change detected timestamp in the user's local timezone. time can't do that in multithreaded applications. There's some kind of UB/unsoundness around getting local time in a thread, which is why this is now printing UTC times.
What do you think of just doing println!("Change detected") and dropping the timestamp? Then the root crate wouldn't need a dependency on time.
There was a problem hiding this comment.
Is it only on MacOS? I kind of like having the datetime in there to make sure I'm getting the changes when I expect them (eg I see nothing changing on the site and I check the the terminal to see if it has been picked up).
At the same time, if it's not showing the local timezone it's not worth it.
It's kinda weird it's not possible though, can we get the user offset on startup before starting any threads and re-using it?
There was a problem hiding this comment.
Seems like that should be possible, I'll look into it.
There was a problem hiding this comment.
That issue indicates it is only on Mac OS but it was happening to me on Fedora. Calling now_local was Err.
There was a problem hiding this comment.
@Keats Grabbing the UTC offset before threading starts worked like a charm. patch pushed to this branch.
There was a problem hiding this comment.
I did have to add another argument to serve(), but thankfully #[allow(clippy::too_many_arguments)] was already there. 😅
| @@ -5,7 +5,7 @@ edition = "2021" | |||
|
|
|||
| [dependencies] | |||
| serde = {version = "1.0", features = ["derive"] } | |||
|
|
|||
| time = { version = "0.3", features = ["macros"] } | |||
There was a problem hiding this comment.
are the macros only used for tests? If so, we should probably remove that dep entirely from this crate and build them manually
There was a problem hiding this comment.
We can't remove the crate, or the macro feature, because they're used in for example, front_matter's parse_datetime:
src/cmd/serve.rs
Outdated
| let format = format_description!("[year]-[month]-[day] [hour]:[minute]:[second] UTC"); | ||
| println!( | ||
| "Change detected @ {}", | ||
| OffsetDateTime::now_utc().format(&format).unwrap().to_string() | ||
| ); |
There was a problem hiding this comment.
Is it only on MacOS? I kind of like having the datetime in there to make sure I'm getting the changes when I expect them (eg I see nothing changing on the site and I check the the terminal to see if it has been picked up).
At the same time, if it's not showing the local timezone it's not worth it.
It's kinda weird it's not possible though, can we get the user offset on startup before starting any threads and re-using it?
Damn looks like we need to fix the Windows CI build soon. |
😬 |
This PR replaces the
chronocrate with thetimecrate.It also makes a subtle internal change in how times are used within Zola. Previously, NaiveDateTime from the chrono crate was used everywhere, which omits timezone offset. If a timezone offset is given to Zola in a post's front matter, it will be flattened into a NaiveDateTime by adding the offset to the datetime. If you tell Zola "this post happened at 4:00+2:00", Zola would convert that to "6:00". Now, the offset is preserved, and UTC is assumed when no offset is provided. The old system was implicitly "convert everything to UTC" and the new system is "assume UTC unless otherwise specified". There's no change in Zola's behavior, though.
Sanity check:
Code changes
(Delete or ignore this section for documentation changes)
nextbranch?If the change is a new feature or adding to/changing an existing one: