Skip to content

Implement builtin setters for Date object #367

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

Closed

Conversation

szledan
Copy link
Contributor

@szledan szledan commented Jul 10, 2015

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

@egavrin egavrin added this to the ECMA builtins milestone Jul 10, 2015
@egavrin egavrin added ecma builtins Related to ECMA built-in routines development Feature implementation labels Jul 10, 2015
extern ecma_completion_value_t ecma_date_set_internal_property (ecma_value_t this_arg,
ecma_number_t day,
ecma_number_t time,
bool input_is_utc);
Copy link
Member

Choose a reason for hiding this comment

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

is_utc is enough

@szledan szledan force-pushed the date_prototype_setters branch from da710f2 to 3171b34 Compare July 14, 2015 08:20
@galpeter galpeter mentioned this pull request Jul 14, 2015
50 tasks
* Used by:
* - All Date.prototype.set* routine except Date.prototype.setTime.
*
* @return completion value
Copy link
Contributor

Choose a reason for hiding this comment

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

@szledan, usually, completion values are returned by specification-defined routines. In the cases, content of completion value is clear, while content of this function's completion is not - without looking into its code.
Could you, please, extend the comment with note about what value is returned from the function, or with a reference to specification, so that origin of the value becomes clear?

@ruben-ayrapetyan
Copy link
Contributor

After updating the comment the changes would look good to me.

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com
@szledan szledan force-pushed the date_prototype_setters branch from 3171b34 to 873987d Compare July 15, 2015 10:58
@galpeter
Copy link
Contributor

looks good to me

@ruben-ayrapetyan ruben-ayrapetyan assigned egavrin and unassigned szledan Jul 15, 2015
@egavrin
Copy link
Contributor

egavrin commented Jul 15, 2015

@ruben-ayrapetyan recheck it please, and I'll merge it if its OK

@ruben-ayrapetyan
Copy link
Contributor

Looks good to me

@egavrin
Copy link
Contributor

egavrin commented Jul 15, 2015

Merged at 005eb04

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Feature implementation ecma builtins Related to ECMA built-in routines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants