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

Add parse_from_* to DateTime #278

Closed
wants to merge 7 commits into from
Closed

Conversation

mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Sep 6, 2018

As discussed in #263. Note that this breaks existing code that did not
explicitly specify the offset in calls to DateTime::parse_from_*, as
they are now ambiguous.

Relies on #271 for conversion back into DateTime<Utc> from
DateTime<FixedOffset> as the latter is used under the hood.

Adds conversion to/from Utc, Local, and FixedOffset. Originally planned
to go through NaiveDateTime, but it seems that is not necessary?
Documentation added to both `impl`s and functions so that they are
visible to both users perusing the online documentation and in
autocomplete/intellisense engines.
As discussed in chronotope#263. Note that this breaks existing code that did not
explicitly specify the offset in calls to `DateTime::parse_from_*`, as
they are now ambiguous.

Relies on chronotope#271 for conversion back into `DateTime<Utc>` from
`DateTime<FixedOffset>` as the latter is used under the hood.
@mqudsi
Copy link
Contributor Author

mqudsi commented Apr 8, 2019

@quodlibetor I don't know if you want to take this alongside #271 as both feature ergonomic improvements to working with timezoned DateTime instances? I've rebased it over the latest version of #271 - it's only one commit on top of that.

@djc djc deleted the branch chronotope:master August 29, 2022 08:43
@djc djc closed this Aug 29, 2022
@mqudsi
Copy link
Contributor Author

mqudsi commented Aug 29, 2022

@djc did you intentionally close this or is that just an artifact of brute forcing "main" onto the repo? Note that the referenced #271 was merged.

@djc
Copy link
Member

djc commented Aug 29, 2022

Nope, just deleted the master branch in response to user confusion.

@mqudsi
Copy link
Contributor Author

mqudsi commented Aug 29, 2022

Ah ok. Do you mind reopening the issue?

or do you need me to create a new pr with this rebased against main?

@djc
Copy link
Member

djc commented Aug 30, 2022

I tried, but was unable to reopen the PR. If you're still interested, please rebase on main.

@mqudsi
Copy link
Contributor Author

mqudsi commented Aug 31, 2022

Re-based and resubmitted as #806.

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