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

Nowutcstuff #15

Merged
merged 3 commits into from
Sep 20, 2014
Merged

Nowutcstuff #15

merged 3 commits into from
Sep 20, 2014

Conversation

IainNZ
Copy link
Contributor

@IainNZ IainNZ commented Sep 20, 2014

Very confused...

@IainNZ
Copy link
Contributor Author

IainNZ commented Sep 20, 2014

@quinnj any ideas why this is necessary? I thought the if at the top should do everything required for Dates stuff?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.38%) when pulling f8a37bb on nowutcstuff into 6f25a1d on master.

@IainNZ
Copy link
Contributor Author

IainNZ commented Sep 20, 2014

@WestleyArgentum maybe you know, you were the one who changed this last (although it wasn't tested)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) when pulling f8a37bb on nowutcstuff into 6f25a1d on master.

@IainNZ
Copy link
Contributor Author

IainNZ commented Sep 20, 2014

I'm merging for now, but still curious if this is correct.

IainNZ added a commit that referenced this pull request Sep 20, 2014
@IainNZ IainNZ merged commit 11f65cb into master Sep 20, 2014
@IainNZ IainNZ deleted the nowutcstuff branch September 20, 2014 19:43
@quinnj
Copy link
Member

quinnj commented Sep 22, 2014

Sorry for not responding quicker. So I think the issue is that Dates.utcnow() was actually removed from 0.3 compatible version (it only ever existed for like, a day, but that's how things go, right?). Was there a 0.3/0.4 compatibility issue with this when just using Dates.now(Dates.UTC)?

@IainNZ
Copy link
Contributor Author

IainNZ commented Sep 22, 2014

Yes there is, at least on Travis.

  • The "fix now(UTC)" commit changed it to Dates.nowutc() which was OK on 0.3, and failed 0.4.
  • The "old behaviour" commit changed it to Dates.now(Dates.UTC) which failed on 0.3, but worked on 0.4
  • The final commit put it in an if statement...

I'm very confused about what should be happening/what is correct.

@WestleyArgentum
Copy link
Collaborator

Sorry! I've been out of town!

Back when I was changing things Dates.nowutc() had just been added to Dates and there was a discussion here: JuliaLang/julia@5999f53 about whether it should be changed. The consensus was to go with Dates.now(Dates.UTC), and I totally dropped the ball changing things once that had been merged.

But I'm a little confused as well, I believe that both 0.3 and 0.4 should both be using the new Dates.now(Dates.UTC) interface. Maybe something didn't get merged back into Dates.jl? Or a new version needs to be tagged? Can poke around in an hour or so

@WestleyArgentum
Copy link
Collaborator

@quinnj It looks like the more recent changes from julia/base/dates need to be pulled into Dates.jl. I'm not 100% sure the easiest way to do this, but it would be nice to keep the same history... so maybe that means making a copy of the julia git repo, stripping everything not ./dates, and then merging that into Dates.jl? I can try sometime soon if no one knows a better way

@quinnj
Copy link
Member

quinnj commented Sep 23, 2014

Yeah, I need to do another sync. I'll try to do it this afternoon.

@quinnj
Copy link
Member

quinnj commented Oct 2, 2014

Ok, synced Dates for 0.3 and tagged new version.

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.

None yet

4 participants