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

Rename tm_gmtoff to tm_utcoff #18832

Closed
wants to merge 1 commit into from
Closed

Conversation

barosl
Copy link
Contributor

@barosl barosl commented Nov 10, 2014

Rename the tm_gmtoff field of time::Tm to tm_utcoff.

Although GMT and UTC are commonly used interchangeably, UTC is more appropriate to denote the time standard.

Python, for example, uses datetime.utcoffset() to represent the offset from UTC.

I think we should fix this while we are allowed to break backward compatibility.

Rename the tm_gmtoff field of time::Tm to tm_utcoff.

Although GMT and UTC are commonly used interchangeably, UTC is more
appropriate to denote the time standard.

Python, for example, uses datetime.utcoffset() to represent the offset
from UTC.

[breaking-change]
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

@brson
Copy link
Contributor

brson commented Nov 10, 2014

Oh, man, I'm not sure what's going on with this library. It's pretty crufty. Tm pretty clearly was derived from - and once was supposed to correspond to - the C tm struct (it has #[repr(C)]), but C's tm doesn't have this field.

Pinging some people who show up in git blame: cc @alexcrichton @erickt @seanmonstar @aturon.

@seanmonstar
Copy link
Contributor

I'd love it if we could re-design the time crate. It is indeed crufty.

As regards this field, the implementation to adjust the gmtoffset is pretty slow as well. See #18725

@aturon
Copy link
Member

aturon commented Nov 10, 2014

@alexcrichton and I discussed this a bit on IRC, and agree that the best way forward is to move libtime entirely out of the rust distro and into its own repo (where it can be redesigned).

The only usages in rust itself are for wall-clock timings, which can be done through the Duration type instead.

@barosl @seanmonstar, if you're interested, take a look at https://mail.mozilla.org/pipermail/rust-dev/2014-July/010956.html for some initial thoughts about this kind of migration. @alexcrichton and I would be glad to help such a migration effort for libtime.

@brson
Copy link
Contributor

brson commented Nov 10, 2014

Should we do this simple patch in the meantime?

@alexcrichton
Copy link
Member

I'm actually going to prepare a PR to remove libtime altogether from the distribution, would you mind sending this PR to https://github.com/rust-lang/time instead?

@alexcrichton
Copy link
Member

PR'd: #18858

@barosl
Copy link
Contributor Author

barosl commented Nov 11, 2014

@alexcrichton I made a pull request on time-rs/time#1. Should I close this?

@alexcrichton
Copy link
Member

I'll go ahead, thanks @barosl!

@barosl barosl deleted the gmtoff-to-utcoff branch November 11, 2014 00:06
@barosl barosl restored the gmtoff-to-utcoff branch November 11, 2014 00:06
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.

6 participants