Skip to content

Conversation

lvidacs
Copy link
Contributor

@lvidacs lvidacs commented Jul 28, 2015

JerryScript-DCO-1.0-Signed-off-by: Laszlo Vidacs [email protected]

@lvidacs
Copy link
Contributor Author

lvidacs commented Jul 28, 2015

The helper function is prepared to handle indexOf() as well, it will be adjusted in a separate step.

Copy link
Member

Choose a reason for hiding this comment

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

Can't we save the iterator itself? Seeking is not a cheap operation.

@dbatyai dbatyai added ecma builtins Related to ECMA built-in routines development Feature implementation labels Jul 28, 2015
@dbatyai dbatyai added this to the ECMA builtins milestone Jul 28, 2015
@lvidacs lvidacs force-pushed the string-prototype-lastindexof branch from 790d120 to 7f758fe Compare July 29, 2015 15:50
@ILyoan ILyoan mentioned this pull request Jul 30, 2015
25 tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

Incorrect indentation

@lvidacs lvidacs force-pushed the string-prototype-lastindexof branch from 7f758fe to a1e0185 Compare July 30, 2015 10:38
@lvidacs
Copy link
Contributor Author

lvidacs commented Jul 30, 2015

@galpeter, thanks for the comments, the patch is rebased and updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the found at all? Can't we just break out?

JerryScript-DCO-1.0-Signed-off-by: Laszlo Vidacs [email protected]
@lvidacs lvidacs force-pushed the string-prototype-lastindexof branch from a1e0185 to 11a6a50 Compare July 30, 2015 13:19
@galpeter
Copy link
Contributor

lgtm

@zherczeg
Copy link
Member

looks good, but please open an issue that where the followup work is described

@lvidacs
Copy link
Contributor Author

lvidacs commented Aug 4, 2015

Issue #515 is opened for follow-up work.

@zherczeg zherczeg assigned egavrin and unassigned lvidacs Aug 4, 2015
@egavrin
Copy link
Contributor

egavrin commented Aug 5, 2015

make push

@egavrin egavrin assigned lvidacs and unassigned egavrin Aug 5, 2015
@kkristof
Copy link
Contributor

kkristof commented Aug 6, 2015

Landed in 554305d

@kkristof kkristof closed this Aug 6, 2015
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.

6 participants