Skip to content

Follow up of API for large years#272

Open
sylvestre wants to merge 9 commits intouutils:mainfrom
sylvestre:follow
Open

Follow up of API for large years#272
sylvestre wants to merge 9 commits intouutils:mainfrom
sylvestre:follow

Conversation

@sylvestre
Copy link
Copy Markdown
Contributor

see #263

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.30%. Comparing base (30ea391) to head (d75b878).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #272   +/-   ##
=======================================
  Coverage   99.29%   99.30%           
=======================================
  Files          20       20           
  Lines        3847     3894   +47     
  Branches      121      122    +1     
=======================================
+ Hits         3820     3867   +47     
  Misses         26       26           
  Partials        1        1           
Flag Coverage Δ
macos_latest 99.30% <100.00%> (+<0.01%) ⬆️
ubuntu_latest 99.30% <100.00%> (+<0.01%) ⬆️
windows_latest 14.04% <0.00%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sylvestre
Copy link
Copy Markdown
Contributor Author

@abhishekpradhan wdyt ? :)

Copy link
Copy Markdown
Contributor

@abhishekpradhan abhishekpradhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. I left one inline note on the new Display behavior, since it seems to drop subsecond precision at the moment, but the rest of the follow-up cleanup makes sense to me. With this work completed, I think #160 is effectively resolved as well. 🙂

Provide a standard formatting path so callers (and tests) can use
`.to_string()` instead of hand-rolling offset formatting everywhere.
Document that expect_in_range is intended for tests/trusted contexts
and that Extended values never compare equal to a Zoned.
The Timestamp variant was #[cfg(test)]-gated dead code outside tests.
The parse_timestamp function already calls set_timestamp on the builder
directly, bypassing the TryFrom<Vec<Item>> path. Remove the variant and
refactor builder tests to exercise set_timestamp directly instead.
Replace hand-rolled format_offset_colon / format_for_assert helpers
with the new Display impls. Replace local expect_in_range_datetime
test helpers with ParsedDateTime::expect_in_range().
Add a dedicated fuzz_large_year target with structured inputs biased
toward large years (near 9999 boundary, up to GNU_MAX_YEAR). Keep the
original fuzz_parse_datetime target unchanged for raw-bytes fuzzing.
When nanoseconds are non-zero, include them in Display output as
`.NNNNNNNNN` between seconds and the offset. This avoids silently
dropping sub-second precision from the formatted representation.
Copy link
Copy Markdown
Contributor

@abhishekpradhan abhishekpradhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, I think we’re good to go here. I left one minor nit on the README example, but otherwise this looks good to me.

```rs
use parse_datetime::{parse_datetime, ParsedDateTime};

let dt = parse_datetime("12000-01-01").unwrap();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this may not always end with +00:00, since it depends on the local timezone.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 31, 2026

Merging this PR will improve performance by 3.6%

⚡ 1 improved benchmark
✅ 16 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
parse_epoch_timestamp 7.5 µs 7.3 µs +3.6%

Comparing sylvestre:follow (d75b878) with main (30ea391)

Open in CodSpeed

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

Successfully merging this pull request may close these issues.

2 participants