Make minutes, seconds optional when deserializing `UtcOffset`

[MaxKingPor]

• Update UTC_OFFSET_FORMAT to accept more formats
• Added visit_i64 and visit_u64 for Visitor<UtcOffset> to accept numeric hours
• Unsafe expose UtcOffset::__from_hms_unchecked as UtcOffset::from_hms_unchecked

[jhpratt]

The only change here that I'm willing to make is having the minutes/seconds being optional.

Changing the padding to be None is a breaking change, and changing the sign to be optional is not ideal, as nearly every format for UTC offsets requires the sign. Adding visit_i64 and visit_u64 is functionally predicated on having the sign be optional.

With regard to having UtcOffset::from_hms_unchecked, there is no precedent for having this in time. Nothing can be constructed in an unchecked manner. If you want this currently, you can call UtcOffset::from_hms followed by Result::unwrap_unchecked.

[MaxKingPor]

Using UtcOffset::from_hms returns the Result type, which is not friendly to building global variables, and I want to maintain the unsafe UtcOffset::from_hms_unchecked directly by the developers themselves

[jhpratt]

Global variables are an antipattern in Rust. If you're looking for a static value, however, you can use the offset! macro. I will not be providing a way to construct a value that can result in undefined behavior. It's simply unnecessary — there's the .unwrap_unchecked() method on results in addition to the macros. Unsafety is a last resort, not something to be reached for on a regular basis.

[MaxKingPor]

Global variables are an antipattern in Rust. If you're looking for a static value, however, you can use the offset! macro. I will not be providing a way to construct a value that can result in undefined behavior. It's simply unnecessary — there's the .unwrap_unchecked() method on results in addition to the macros. Unsafety is a last resort, not something to be reached for on a regular basis.

done

[jhpratt]

CI is currently failing in a few ways. You'll almost certainly need to update the tests.

Also, don't just do .unwrap_or(0). This makes the assumption that if deserializing minutes fails, deserializing seconds will fail as well. While that is likely the case, it's not guaranteed.

[codecov[bot]]

Codecov Report

Merging #588 (a630c08) into main (b5cf98c) will increase coverage by 0.0%. The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #588   +/-   ##
=====================================
  Coverage   95.8%   95.8%           
=====================================
  Files         79      79           
  Lines       8809    8815    +6     
=====================================
+ Hits        8435    8441    +6     
  Misses       374     374           

Impacted Files	Coverage Δ
time-macros/src/lib.rs	72.5% <ø> (ø)
time/src/serde/mod.rs	96.5% <ø> (ø)
time/src/serde/visitor.rs	100.0% <100.0%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[MaxKingPor]

CI is currently failing in a few ways. You'll almost certainly need to update the tests.

Also, don't just do .unwrap_or(0). This makes the assumption that if deserializing minutes fails, deserializing seconds will fail as well. While that is likely the case, it's not guaranteed.

done

[jhpratt]

Thanks. I'll take a look at the PR in the coming days.