Conversation
charliermarsh
approved these changes
Aug 19, 2024
Member
|
Since e change interpretation of exclude newer times this is "breaking", right? |
Member
Author
Yup. |
Member
Author
|
This is also breaking because uv won't parse old lock files either, since the datetime formats won't match. |
Member
|
Let's just group it into 0.3.0, we might start merging things for that today? I'll let you know. |
konstin
added a commit
that referenced
this pull request
Aug 19, 2024
Update reqwest-middleware to the latest upstream (https://github.com/TrueLayer/reqwest-middleware, 603ef97144b6b328c4e9ef7b13297d40bf461779) for #6205.
BurntSushi
pushed a commit
that referenced
this pull request
Aug 19, 2024
Update reqwest-middleware to the latest upstream (https://github.com/TrueLayer/reqwest-middleware, 603ef97144b6b328c4e9ef7b13297d40bf461779) for #6205.
Member
Author
|
OK, with #6206 merged, Chrono is now removed from
SGTM. |
The port was pretty straight-forward. I think the only interesting change is in the parsing for `--exclude-newer`, but the existing behavior shouldn't change (it's still RFC 3339 compatible) and the error messages I think have improved.
Previously, `uv` was writing a non-standard timestamp format to the lock file. Jiff could be made to write this same format, but it actually seems a lot better to use something standardized. And the RFC 3339 format with the Zulu offset is the right thing for this.
Previously, `--exclude-newer` would accept either an RFC 3339 timestamp or a "UTC date." A UTC date is a somewhat weird interaction point, because it requires the end user to think about what date it is at offset 0, which may or may not match their actual date. Instead, we just accept a "local" date which will be interpreted using their system's current time zone. An alternative here would be to get rid of the date option and always require a RFC 3339 timestamp.
zanieb
pushed a commit
that referenced
this pull request
Aug 19, 2024
This PR migrates uv's use of `chrono` to `jiff`. I did most of this work a while back as one of my tests to ensure Jiff could actually be used in a real world project. I decided to revive this because I noticed that `reqwest-retry` dropped its Chrono dependency, which is I believe the only other thing requiring Chrono in uv. (Although, we use a fork of `reqwest-middleware` at present, and that hasn't been updated to latest upstream yet. I wasn't quite sure of the process we have for that.) In course of doing this, I actually made two changes to uv: First is that the lock file now writes an RFC 3339 timestamp for `exclude-newer`. Previously, we were using Chrono's `Display` implementation for this which is a non-standard but "human readable" format. I think the right thing to do here is an RFC 3339 timestamp. Second is that, in addition to an RFC 3339 timestamp, `--exclude-newer` used to accept a "UTC date." But this PR changes it to a "local date." That is, a date in the user's system configured time zone. I think this makes more sense than a UTC date, but one alternative is to drop support for a date and just rely on an RFC 3339 timestamp. The main motivation here is that automatically assuming UTC is often somewhat confusing, since just writing an unqualified date like `2024-08-19` is often assumed to be interpreted relative to the writer's "local" time.
zanieb
pushed a commit
that referenced
this pull request
Aug 19, 2024
This PR migrates uv's use of `chrono` to `jiff`. I did most of this work a while back as one of my tests to ensure Jiff could actually be used in a real world project. I decided to revive this because I noticed that `reqwest-retry` dropped its Chrono dependency, which is I believe the only other thing requiring Chrono in uv. (Although, we use a fork of `reqwest-middleware` at present, and that hasn't been updated to latest upstream yet. I wasn't quite sure of the process we have for that.) In course of doing this, I actually made two changes to uv: First is that the lock file now writes an RFC 3339 timestamp for `exclude-newer`. Previously, we were using Chrono's `Display` implementation for this which is a non-standard but "human readable" format. I think the right thing to do here is an RFC 3339 timestamp. Second is that, in addition to an RFC 3339 timestamp, `--exclude-newer` used to accept a "UTC date." But this PR changes it to a "local date." That is, a date in the user's system configured time zone. I think this makes more sense than a UTC date, but one alternative is to drop support for a date and just rely on an RFC 3339 timestamp. The main motivation here is that automatically assuming UTC is often somewhat confusing, since just writing an unqualified date like `2024-08-19` is often assumed to be interpreted relative to the writer's "local" time.
zanieb
pushed a commit
that referenced
this pull request
Aug 19, 2024
This PR migrates uv's use of `chrono` to `jiff`. I did most of this work a while back as one of my tests to ensure Jiff could actually be used in a real world project. I decided to revive this because I noticed that `reqwest-retry` dropped its Chrono dependency, which is I believe the only other thing requiring Chrono in uv. (Although, we use a fork of `reqwest-middleware` at present, and that hasn't been updated to latest upstream yet. I wasn't quite sure of the process we have for that.) In course of doing this, I actually made two changes to uv: First is that the lock file now writes an RFC 3339 timestamp for `exclude-newer`. Previously, we were using Chrono's `Display` implementation for this which is a non-standard but "human readable" format. I think the right thing to do here is an RFC 3339 timestamp. Second is that, in addition to an RFC 3339 timestamp, `--exclude-newer` used to accept a "UTC date." But this PR changes it to a "local date." That is, a date in the user's system configured time zone. I think this makes more sense than a UTC date, but one alternative is to drop support for a date and just rely on an RFC 3339 timestamp. The main motivation here is that automatically assuming UTC is often somewhat confusing, since just writing an unqualified date like `2024-08-19` is often assumed to be interpreted relative to the writer's "local" time.
zanieb
pushed a commit
that referenced
this pull request
Aug 20, 2024
This PR migrates uv's use of `chrono` to `jiff`. I did most of this work a while back as one of my tests to ensure Jiff could actually be used in a real world project. I decided to revive this because I noticed that `reqwest-retry` dropped its Chrono dependency, which is I believe the only other thing requiring Chrono in uv. (Although, we use a fork of `reqwest-middleware` at present, and that hasn't been updated to latest upstream yet. I wasn't quite sure of the process we have for that.) In course of doing this, I actually made two changes to uv: First is that the lock file now writes an RFC 3339 timestamp for `exclude-newer`. Previously, we were using Chrono's `Display` implementation for this which is a non-standard but "human readable" format. I think the right thing to do here is an RFC 3339 timestamp. Second is that, in addition to an RFC 3339 timestamp, `--exclude-newer` used to accept a "UTC date." But this PR changes it to a "local date." That is, a date in the user's system configured time zone. I think this makes more sense than a UTC date, but one alternative is to drop support for a date and just rely on an RFC 3339 timestamp. The main motivation here is that automatically assuming UTC is often somewhat confusing, since just writing an unqualified date like `2024-08-19` is often assumed to be interpreted relative to the writer's "local" time.
zanieb
pushed a commit
that referenced
this pull request
Aug 20, 2024
This PR migrates uv's use of `chrono` to `jiff`. I did most of this work a while back as one of my tests to ensure Jiff could actually be used in a real world project. I decided to revive this because I noticed that `reqwest-retry` dropped its Chrono dependency, which is I believe the only other thing requiring Chrono in uv. (Although, we use a fork of `reqwest-middleware` at present, and that hasn't been updated to latest upstream yet. I wasn't quite sure of the process we have for that.) In course of doing this, I actually made two changes to uv: First is that the lock file now writes an RFC 3339 timestamp for `exclude-newer`. Previously, we were using Chrono's `Display` implementation for this which is a non-standard but "human readable" format. I think the right thing to do here is an RFC 3339 timestamp. Second is that, in addition to an RFC 3339 timestamp, `--exclude-newer` used to accept a "UTC date." But this PR changes it to a "local date." That is, a date in the user's system configured time zone. I think this makes more sense than a UTC date, but one alternative is to drop support for a date and just rely on an RFC 3339 timestamp. The main motivation here is that automatically assuming UTC is often somewhat confusing, since just writing an unqualified date like `2024-08-19` is often assumed to be interpreted relative to the writer's "local" time.
zanieb
pushed a commit
that referenced
this pull request
Aug 20, 2024
This PR migrates uv's use of `chrono` to `jiff`. I did most of this work a while back as one of my tests to ensure Jiff could actually be used in a real world project. I decided to revive this because I noticed that `reqwest-retry` dropped its Chrono dependency, which is I believe the only other thing requiring Chrono in uv. (Although, we use a fork of `reqwest-middleware` at present, and that hasn't been updated to latest upstream yet. I wasn't quite sure of the process we have for that.) In course of doing this, I actually made two changes to uv: First is that the lock file now writes an RFC 3339 timestamp for `exclude-newer`. Previously, we were using Chrono's `Display` implementation for this which is a non-standard but "human readable" format. I think the right thing to do here is an RFC 3339 timestamp. Second is that, in addition to an RFC 3339 timestamp, `--exclude-newer` used to accept a "UTC date." But this PR changes it to a "local date." That is, a date in the user's system configured time zone. I think this makes more sense than a UTC date, but one alternative is to drop support for a date and just rely on an RFC 3339 timestamp. The main motivation here is that automatically assuming UTC is often somewhat confusing, since just writing an unqualified date like `2024-08-19` is often assumed to be interpreted relative to the writer's "local" time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR migrates uv's use of
chronotojiff.I did most of this work a while back as one of my tests to ensure Jiff
could actually be used in a real world project. I decided to revive
this because I noticed that
reqwest-retrydropped its Chrono dependency,which is I believe the only other thing requiring Chrono in uv.
(Although, we use a fork of
reqwest-middlewareat present, and thathasn't been updated to latest upstream yet. I wasn't quite sure of the
process we have for that.)
In course of doing this, I actually made two changes to uv:
First is that the lock file now writes an RFC 3339 timestamp for
exclude-newer. Previously, we were using Chrono'sDisplayimplementation for this which is a non-standard but "human readable"
format. I think the right thing to do here is an RFC 3339 timestamp.
Second is that, in addition to an RFC 3339 timestamp,
--exclude-newerused to accept a "UTC date." But this PR changes it to a "local date."
That is, a date in the user's system configured time zone. I think
this makes more sense than a UTC date, but one alternative is to drop
support for a date and just rely on an RFC 3339 timestamp. The main
motivation here is that automatically assuming UTC is often somewhat
confusing, since just writing an unqualified date like
2024-08-19isoften assumed to be interpreted relative to the writer's "local" time.