-
Notifications
You must be signed in to change notification settings - Fork 463
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
Switch from the chrono clock
feature to now
#1569
Conversation
|
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
65244a9
to
0320780
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! ❤️
The
|
We have a bug open for it, it's a flaky test, don't worry. |
There were no conflicts from this merge. We were still waiting on the no defaults PR #1562 at the time, so even if there were conflicts it would have been 1 small line diff and as this is a leaf had no downstream affects on other crates. In general, I agree we need to establish some sort of process, but there was only a net good considering how often we get asks for releases. |
Yes. I only meant to re-highlight the need to establish a process. Fully agree with your reasoning for merging this. |
In #1192,
chrono
was added as a dependency of theopentelemetry-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
v0.4.32 has added support for an even finer-grained feature namednow
, which is a subset of theclock
feature - that excludes timezone support, and so avoids pulling in many timezone related crates.opentelemetry-stdout
only uses chrono's UTC features, so we can safely switch from using theclock
feature to usingnow
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