Using time crate instead of chrono #533
Conversation
… time-humanize, refactored code to display creaton of repo time
HallerPatrick
added
discussion
dependencies
|
Looks like the audit finds a soundness issue for |
| fn git_time_to_formatted_time(time: &Time, iso_time: bool) -> String { | ||
| let (offset, _) = match time.offset_minutes() { | ||
| n if n < 0 => (-n, '-'), | ||
| n => (n, '+'), | ||
| }; | ||
|
|
||
| let offset = FixedOffset::west(offset); | ||
| let dt_with_tz = offset.timestamp(time.seconds(), 0); | ||
| if iso_time { | ||
| dt_with_tz | ||
| .with_timezone(&chrono::Utc) | ||
| .to_rfc3339_opts(chrono::SecondsFormat::Secs, true) | ||
| to_rfc3339(HumanTime::from(time.seconds())) | ||
| } else { | ||
| let ht = HumanTime::from(dt_with_tz); | ||
| let ht = HumanTime::from_duration_since_timestamp(time.seconds().unsigned_abs()); | ||
| format!("{}", ht) | ||
| } | ||
| } | ||
|
|
||
| fn to_rfc3339<T>(dt: T) -> String |
There was a problem hiding this comment.
It might be cool if we added some tests for these functions. Unfortunately we didn't have any prior tests for duration formatting to confirm consistent behavior.
There was a problem hiding this comment.
Yes, can do that
There was a problem hiding this comment.
@spenserblack You think those tests are sufficient?
Co-authored-by: Spenser Black <[email protected]>
|
Sorry @HallerPatrick, I gave you a bad suggestion. See #533 (comment) |
In the audit there is a reference link, which leads to the |
|
Got it, thanks for linking the issue! tl;dr it doesn't seem like we have to worry about |
|
@o2sh does not seem to be active currently. Because I got your okay, I would merge this now |
|
So sorry I couldn't find the time to review your PR @HallerPatrick 😞 Everything seems to work as expected. My only concern is the Rust version 1.56.1 is very recent and not yet available for launchpad |
No problem at all. @spenserblack gave good feedback. For |
|
@o2sh I updated the rust version of |
|
Thanks @HallerPatrick 👍 |
This PR solves Issue #526.
I ported, more or less, the
chrono-humanizelibrary to only use the std lib time and as a optional feature thetime-rscrate ( for display iso time). Because I wanted to keep most of tests of the original repo (which are a lot!) and also included some nice interface, I made this an own crate.The code now even looks way cleaner and include less logic. I tested the human readable output as well as the iso output on different repos. Feel free to test this as well.