Comments below, otherwise LGTM. -Ivan
http://codereview.chromium.org/63100/diff/1/3 File test/mjsunit/array-shift.js (right): http://codereview.chromium.org/63100/diff/1/3#newcode29 Line 29: arr = [1, 2, 3, 4, 5]; The fact that the array indices and the values do not line up is rather confusing. To distinguish indices and values, can you use ["a", "b", "c", "d", "e"]? Or if you prefer not to use strings then use variables var a = 10; var b = 20; ... arr = [a, b, c, d, e]; In any case it would more clearly express your intentions if you used a different value range for the array contents at least: arr = [100, 101, 102, 103, 104, 105]; http://codereview.chromium.org/63100/diff/1/3#newcode41 Line 41: assertArrayEquals([2,,4,,], arr); Do you need the extra comma here before the closing bracket? http://codereview.chromium.org/63100/diff/1/3#newcode87 Line 87: assertFalse(1999 in arr); How about also asserting that the indices between 2000 and 2100 are in the array before the splicing operations? Similarly for the next chunks of missing and available indices. http://codereview.chromium.org/63100/diff/1/2 File test/mjsunit/array-splice-webkit.js (right): http://codereview.chromium.org/63100/diff/1/2#newcode61 Line 61: arr = [1,2,3,4,5]; Same concern about mixing indices and values... http://codereview.chromium.org/63100 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
