Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split clock feature into clock and now #1343

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Oct 28, 2023

Fixes #1341

Split a now feature off from clock that enables Utc::now() only. This function is useful on its own, and can be included without the small-but-measurable overhead of adding timezone support.

For future compatibility, the feature clock implies the feature now.

Build matrix

A now-only build run was added to the matrix, though this might be overkill.

Tests

This PR updates a few tests to ensure that --features now and --feature clock both continue work. I added a hidden cfg to one test to ensure that it can pass in both clock and now feature sets.

Tested the following builds successfully:

cargo test
cargo test --no-default-features --features=now
cargo test --no-default-features --features=clock

@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

Merging #1343 (da4106c) into 0.4.x (dd3f2f8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##            0.4.x    #1343   +/-   ##
=======================================
  Coverage   91.61%   91.61%           
=======================================
  Files          38       38           
  Lines       17482    17489    +7     
=======================================
+ Hits        16016    16023    +7     
  Misses       1466     1466           
Files Coverage Δ
src/lib.rs 95.80% <100.00%> (+0.21%) ⬆️
src/offset/utc.rs 84.21% <ø> (ø)

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@mmastrac
Copy link
Contributor Author

mmastrac commented Oct 28, 2023

One workflow run appears to be failing on a hashbrown dependency which doesn't make sense. I'll dig into it.

EDIT: it looks like the MSRV test is failing on other PRs as well.

@mmastrac mmastrac force-pushed the split_clock_feature branch from 83ada6e to da4106c Compare November 7, 2023 22:17
@djc djc merged commit c0f418b into chronotope:0.4.x Nov 8, 2023
37 checks passed
@djc
Copy link
Member

djc commented Nov 8, 2023

Thanks!

edmorley added a commit to edmorley/opentelemetry-rust that referenced this pull request Feb 23, 2024
In open-telemetry#1192, `chrono` was added as a dependency of the `opentelemetry-stdout`
crate in order to support outputting timestamps in human readable format.

In that PR, all Chrono features were disabled apart from the `clock` feature.

However, since that change landed, `chrono` has added support for an even
finer-grained feature named `now`, which is a subset of the `clock` feature which
excludes timezone support, and so avoids pulling in many timezone related crates.

`opentelemetry-stdout` only uses chrono's UTC features, so we can switch
from using the `clock` feature to using `now` instead.

After this change, the following transitive dependencies are no longer pulled in:

- `android-tzdata`
- `android_system_properties`
- `cc`
- `core-foundation-sys`
- `iana-time-zone`
- `iana-time-zone-haiku`
- `windows-core`
- `windows-targets`
- `windows_aarch64_gnullvm`
- `windows_aarch64_msvc`
- `windows_i686_gnu`
- `windows_i686_msvc`
- `windows_x86_64_gnu`
- `windows_x86_64_gnullvm`
- `windows_x86_64_msvc`

See:
chronotope/chrono#1343
https://github.com/chronotope/chrono/blob/main/README.md#crate-features
edmorley added a commit to edmorley/opentelemetry-rust that referenced this pull request Feb 23, 2024
In open-telemetry#1192, `chrono` was added as a dependency of the `opentelemetry-stdout`
crate in order to support outputting timestamps in human readable format.

In that PR, all Chrono features were disabled apart from the `clock` feature.

However, since that change landed, `chrono` has added support for an even
finer-grained feature named `now`, which is a subset of the `clock` feature which
excludes timezone support, and so avoids pulling in many timezone related crates.

`opentelemetry-stdout` only uses chrono's UTC features, so we can switch
from using the `clock` feature to using `now` instead.

After this change, the following transitive dependencies are no longer pulled in:

- `android-tzdata`
- `android_system_properties`
- `cc`
- `core-foundation-sys`
- `iana-time-zone`
- `iana-time-zone-haiku`
- `windows-core`
- `windows-targets`
- `windows_aarch64_gnullvm`
- `windows_aarch64_msvc`
- `windows_i686_gnu`
- `windows_i686_msvc`
- `windows_x86_64_gnu`
- `windows_x86_64_gnullvm`
- `windows_x86_64_msvc`

See:
chronotope/chrono#1343
https://github.com/chronotope/chrono/blob/main/README.md#crate-features
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