Skip to content

Implement Date object helper functions #199

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 5 commits into from
Closed

Implement Date object helper functions #199

wants to merge 5 commits into from

Conversation

@szledan
Copy link
Contributor

@szledan szledan commented Jun 17, 2015

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com

@LaszloLango LaszloLango added ecma builtins Related to ECMA built-in routines development Feature implementation labels Jun 17, 2015
@LaszloLango LaszloLango added this to the ECMA builtins milestone Jun 17, 2015
dbatyai
dbatyai reviewed Jun 17, 2015
View reviewed changes
jerry-core/ecma/base/ecma-helpers-date.cpp Outdated
Copy link
Member

@dbatyai dbatyai Jun 17, 2015

Choose a reason for hiding this comment

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

This should be 2015 only.

@dbatyai
Copy link
Member

dbatyai commented Jun 17, 2015

Otherwise lgtm.

egavrin
egavrin reviewed Jun 17, 2015
View reviewed changes
third-party/fdlibm/s_fmod.c Outdated
Copy link
Contributor

@egavrin egavrin Jun 17, 2015

Choose a reason for hiding this comment

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

Could you, please, commit addition of ./third-party/fdlibm/* separately.

ruben-ayrapetyan
ruben-ayrapetyan reviewed Jun 18, 2015
View reviewed changes
jerry-core/ecma/base/ecma-helpers-date.cpp Outdated
Copy link

@ruben-ayrapetyan ruben-ayrapetyan Jun 18, 2015

Choose a reason for hiding this comment

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

Are helpers from the module supposed to be used somewhere outside of jerry-core/ecma/builtin-objects/?

Copy link
Contributor Author

@szledan szledan Jun 19, 2015

Choose a reason for hiding this comment

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

No, maybe we wouldn't use them anywhere outside of builtins. So should I remove this guard?

Copy link

@ruben-ayrapetyan ruben-ayrapetyan Jun 19, 2015

Choose a reason for hiding this comment

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

Please, move them to jerry-core/ecma/builtin-objects (maybe to ecma-builtin-helpers.cpp, as the helpers would be used for implementation of both Date and Date.prototype built-ins).

@szledan szledan mentioned this pull request Jun 19, 2015
Closed
@egavrin
Copy link
Contributor

egavrin commented Jun 21, 2015

Please, split PR on two commits: Add fdlib and Date logic.

@szledan szledan force-pushed the date_helpers branch 2 times, most recently from d8fcbbb to 3e960da Compare June 22, 2015 08:35
@szledan
Copy link
Contributor Author

szledan commented Jun 22, 2015

This PR depends on #223.

szledan added 4 commits June 22, 2015 13:38
@szledan
Implement Date object helper functions
6a983e8 
JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com
@szledan
Implement missing functions in ecma-helpers-date.cpp
55c8704 
JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com
@szledan
Add NaN checking asserts to the Date helper functions
d9b5f5f 
JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com
@szledan
Move helper functions to the builtin section
16aa646 
JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com
galpeter
galpeter reviewed Jun 26, 2015
View reviewed changes
jerry-core/ecma/builtin-objects/ecma-builtin-helpers-date.cpp Outdated
Copy link
Contributor

@galpeter galpeter Jun 26, 2015

Choose a reason for hiding this comment

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

Minor note: there is no need to the function name to the unimplemented comment as the macro will print out the file name, function name and line number.

@szledan
Style fix
c2c1967 
Add comments to the time range defines

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com
@szledan
Copy link
Contributor Author

szledan commented Jun 29, 2015

Hi, I have updated the patch. Could you take a look? ( @kkristof )

@kkristof
Copy link
Contributor

kkristof commented Jun 29, 2015

lgtm

@ruben-ayrapetyan
Copy link

ruben-ayrapetyan commented Jun 29, 2015

Looks good to me

@galpeter
Copy link
Contributor

galpeter commented Jun 29, 2015

Rebased & merged: 07148d3

@galpeter galpeter closed this Jun 29, 2015
@zherczeg zherczeg mentioned this pull request Jul 20, 2016
Closed
This was referenced May 17, 2020
Closed
Closed
This was referenced Oct 20, 2020
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@szledan szledan

Labels

development Feature implementation ecma builtins Related to ECMA built-in routines

Projects

None yet

Milestone

ECMA builtins

Development

Successfully merging this pull request may close these issues.

8 participants

@szledan @dbatyai @egavrin @kkristof @ruben-ayrapetyan @galpeter @sand1k @LaszloLango