Chromium Code Reviews
Help | Chromium Project | Sign in
(25)

Issue 5773002: Fix issue 965. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by Lasse Reichstein
Modified:
3 years, 9 months ago
Reviewers:
Kevin Millikin, Rico
CC:
v8-dev
Visibility:
Public.

Description

Fix issue 965.

Patch Set 1 #

Total comments: 7

Patch Set 2 : Address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -1 line) Patch
M src/v8natives.js View 1 1 chunk +7 lines, -1 line 0 comments Download
M test/mjsunit/object-define-property.js View 1 1 chunk +32 lines, -0 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 6 (0 generated)
Lasse Reichstein
4 years, 3 months ago (2010-12-10 10:29:33 UTC) #1
Rico
LGTM http://codereview.chromium.org/5773002/diff/1/src/v8natives.js File src/v8natives.js (right): http://codereview.chromium.org/5773002/diff/1/src/v8natives.js#newcode618 src/v8natives.js:618: var value = Maybe add a comment that ...
4 years, 3 months ago (2010-12-10 10:42:35 UTC) #2
Kevin Millikin
Drive by. http://codereview.chromium.org/5773002/diff/1/src/v8natives.js File src/v8natives.js (right): http://codereview.chromium.org/5773002/diff/1/src/v8natives.js#newcode618 src/v8natives.js:618: var value = I would actually suggest ...
4 years, 3 months ago (2010-12-10 11:08:23 UTC) #3
Lasse Reichstein
http://codereview.chromium.org/5773002/diff/1/src/v8natives.js File src/v8natives.js (right): http://codereview.chromium.org/5773002/diff/1/src/v8natives.js#newcode618 src/v8natives.js:618: var value = Changed to: var value; // Default ...
4 years, 3 months ago (2010-12-10 11:21:45 UTC) #4
Kevin Millikin
http://codereview.chromium.org/5773002/diff/1/src/v8natives.js File src/v8natives.js (right): http://codereview.chromium.org/5773002/diff/1/src/v8natives.js#newcode618 src/v8natives.js:618: var value = I still think that's too cute. ...
4 years, 3 months ago (2010-12-10 11:28:20 UTC) #5
Lasse Reichstein
4 years, 3 months ago (2010-12-10 11:37:51 UTC) #6
http://codereview.chromium.org/5773002/diff/1/src/v8natives.js
File src/v8natives.js (right):

http://codereview.chromium.org/5773002/diff/1/src/v8natives.js#newcode618
src/v8natives.js:618: var value =
True, good point.
I'll make a new CL and make it 
 var value = void 0;
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ecdb341-tainted