Skip to content

cross platform: Enable DateTime #915

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

cross platform: Enable DateTime #915

wants to merge 2 commits into from

Conversation

@obastemur
Copy link
Collaborator

@obastemur obastemur commented May 3, 2016 •
edited
Loading

This PR enables DateTime

dilijev
dilijev reviewed May 4, 2016
View reviewed changes
lib/Runtime/Library/JavascriptNumber.h Outdated
// and tagged float now has top 14 XOR'd with 1s - the value with 14 bits non zero is
// a float.
// A float can have all 14 bits 0 iff it was a NaN in the first place. Since
// A float can have all 14 bits 0 if it was a NaN in the first place. Since
Copy link
Contributor

@dilijev dilijev May 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"iff" means "if and only if" -- is that not correct?

Copy link
Collaborator Author

@obastemur obastemur May 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh looked like a typo instead. wanted to fix it on the way.

reverted this back. PR is updated.

@dilijev
Copy link
Contributor

dilijev commented May 4, 2016

LGTM but wait for another sign off as well.

curtisman
curtisman reviewed May 4, 2016
View reviewed changes
lib/Runtime/Base/HiResTimer.cpp Outdated
// bring it inline with a time server.
double HiResTimer::GetAdjustFactor()
{
#ifndef _MSC_VER
Copy link
Contributor

@curtisman curtisman May 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be OS dependent instead of compiler dependent?

@digitalinfinity
Copy link
Contributor

digitalinfinity commented May 5, 2016

LGTM as well, but echo @curtisman's comments

@obastemur
Copy link
Collaborator Author

obastemur commented May 5, 2016

PR is updated. @dilijev @curtisman @digitalinfinity thanks for the review

@obastemur obastemur mentioned this pull request May 5, 2016
Closed
@digitalinfinity
Copy link
Contributor

digitalinfinity commented May 5, 2016

@dotnet-bot test this please

@jianchun
Copy link

jianchun commented May 6, 2016

@dotnet-bot test Ubuntu ubuntu_linux_debug please
@dotnet-bot test Ubuntu ubuntu_linux_release please
@dotnet-bot test Windows arm_test please

@obastemur obastemur changed the title cross platform: Enable HiResTimer cross platform: Enable DateTime May 6, 2016
@obastemur obastemur mentioned this pull request May 12, 2016
Merged
@obastemur obastemur closed this May 12, 2016
chakrabot pushed a commit that referenced this pull request May 25, 2016
@obastemur
[MERGE #975] cross-platform: DateTime Implementation
9913c41 
Merge pull request #975 from obastemur:date_work
- Implements DateTime support Cross-Platform
- Redesigns DaylightHelper and Date utilities
- Replaces #915
@obastemur obastemur deleted the date_now branch June 7, 2016 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@obastemur @dilijev @digitalinfinity @jianchun @curtisman @msftclas