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

implement support for time-rs 0.2 #115

Closed
Silentdoer opened this issue Feb 25, 2020 · 5 comments
Closed

implement support for time-rs 0.2 #115

Silentdoer opened this issue Feb 25, 2020 · 5 comments

Comments

@Silentdoer
Copy link

@Silentdoer Silentdoer commented Feb 25, 2020

like title

@abonander

This comment has been minimized.

Copy link
Contributor

@abonander abonander commented Feb 25, 2020

This was a planned feature that we forgot to add to the tracker when we went public, thank you for the reminder.

We would have had this done for 0.1 but we hesitated because we initially thought the time feature would have to be mutually exclusive with the existing chrono feature for sqlx-macros to work correctly (and we don't like mutually exclusive features, one of the reasons it took us so long to add support for Tokio). However, @mehcode and I now agree that sqlx-macros can and should be coded so that it simply prefers to use types from time if both the time and chrono features are activated.

This would actually be a good medium-difficulty issue for a new contributor to take on and get a feel for the codebase. The implementation should be relatively straightforward to map over from the chrono code, and then the above described change to sqlx-macros would be to mark the chrono types with #[cfg(all(feature = "chrono", not(feature = "time")))] and add listings for the equivalent time types with #[cfg(feature = "time")] in the following files:

@abonander abonander changed the title could you support the time-rs 0.2.7? implement support for time-rs 0.2 Feb 25, 2020
@utter-step

This comment has been minimized.

Copy link
Contributor

@utter-step utter-step commented Feb 27, 2020

Hi!

I'd like to try implementing this as one of my first rusty contributions :)
I think of doing this (or maybe it'd be more realistic to say "start doing this"..) in the nearest weekends. Would that be OK?

@abonander

This comment has been minimized.

Copy link
Contributor

@abonander abonander commented Feb 27, 2020

Be our guest! If you get stuck, don't be afraid to ask for help.

Let us know when you get started on it in earnest; we may end up needing this feature for use in an internal project and end up implementing ourselves, but I think we can wait until you've had a chance to work on it.

@utter-step

This comment has been minimized.

Copy link
Contributor

@utter-step utter-step commented Feb 29, 2020

Hello again!

Started working on this issue, hope to send PR today's evening (or at some point tomorrow, if something goes wrong, I'll keep you updated)

utter-step added a commit to utter-step/sqlx that referenced this issue Feb 29, 2020
@utter-step

This comment has been minimized.

Copy link
Contributor

@utter-step utter-step commented Mar 1, 2020

PR: #119

@utter-step utter-step mentioned this issue Mar 1, 2020
4 of 4 tasks complete
@mehcode mehcode closed this in #119 Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.