SpaceManiac / discord-rs

Rust library for the Discord chat client API
MIT License
387 stars 94 forks source link

Removed use of 'time' #132

Closed jD91mZM2 closed 7 years ago

jD91mZM2 commented 7 years ago

This fixes #130.

I do wonder however... Is this really the best solution? I feel like chrono is a huge library taking care of leap seconds and all that good stuff, but maybe I should be using the built in std::time instead?

You decide what you think is best!

Warning: It compiles, but functionality untested. And even if I did test it I wouldn't know what signs to look for that it worked.

Xaeroxe commented 7 years ago

@legolord208 Chrono has much better support for manipulation of time and duration values. I'm in favor of its continued usage.

SpaceManiac commented 7 years ago

+1 for chrono on creation_date, that's a good unification of public API (since everything else exposes chrono types).

Now, it looks like this code was wrong before, but if we're already here it should probably be fixed - Timer should probably be using monotonic std::time::Instant rather than a timestamp, to guard against the clock going backwards. This code was originally written before it was stable.

Ratelimit tracking using UTC is correct. Long-term I want it to use Instant too and never compare our clock to the server's clock directly, but that's rather involved and out of scope here. I don't expect it in this PR.

jD91mZM2 commented 7 years ago

Will work on the features later. Happy Rust 1.19 by the way :smile:

jD91mZM2 commented 7 years ago

There you go! You have no idea how much I suddenly dislike the auto-formatting feature of the text editor I use haha :stuck_out_tongue:

SpaceManiac commented 7 years ago

Looks good as a first pass. Couple of changes here I want to test first so I'll post back after that.

jD91mZM2 commented 7 years ago

Check out #133 also. It adds Travis CI: Automated testing!

jD91mZM2 commented 7 years ago

Hmm it panicked? Type bounds? Because I just translated the existing code. I didn't edit any logic...

SpaceManiac commented 7 years ago

std::time::Duration is unsigned compared to time::Duration.

jD91mZM2 commented 7 years ago

o ok