Issue 1581033002: [es7] implement Object.values() Code Review (original) (raw)
Description
Patch Set 1#
Total comments: 4
Patch Set 2 : change the flag to --harmony-object-values-entries#
Patch Set 3 : add computed property names#
Patch Set 4 : rebase#
Patch Set 5 : Rebase#
Patch Set 6 : Execution::ToObject() -> Object::ToObject(), test fixups#
Messages
Total messages: 27 (14 generated)
| caitp (gmail) | low priority as noted on the bug#, but maybe it will give someone something to ... | 4 years, 11 months ago (2016-01-13 04:45:16 UTC)#1 |
|---|
| rossberg | LGTM with nits https://codereview.chromium.org/1581033002/diff/1/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/1581033002/diff/1/src/flag-definitions.h#newcode210 src/flag-definitions.h:210: V(harmony_object_values, "harmony Object.values") Nit: ...and entries. ... | 4 years, 11 months ago (2016-01-14 13🔞43 UTC)#2 |
|---|
| caitp (gmail) | https://codereview.chromium.org/1581033002/diff/1/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/1581033002/diff/1/src/flag-definitions.h#newcode210 src/flag-definitions.h:210: V(harmony_object_values, "harmony Object.values") On 2016/01/14 13🔞43, rossberg wrote: > ... | 4 years, 11 months ago (2016-01-14 14:13:25 UTC)#3 |
|---|
| caitp (gmail) | Needs an LGTM for src/heap as well. +ulan? | 4 years, 11 months ago (2016-01-14 15:47:48 UTC)#4 |
|---|
| caitp (gmail) | Description was changed from ========== [es7] implement Object.values() / Object.entries() proposal BUG=v8:4663 LOG=N R=ljharb@gmail.com, rossberg@chromium.org, ... | 4 years, 11 months ago (2016-01-14 15:47:59 UTC)#5 |
|---|
| caitp (gmail) | caitpotter88@gmail.com changed reviewers: + ulan@chromium.org | 4 years, 11 months ago (2016-01-14 15:47:59 UTC)#6 |
|---|
| Dan Ehrenberg | Description was changed from ========== [es7] implement Object.values() / Object.entries() proposal BUG=v8:4663 LOG=N R=ljharb@gmail.com, rossberg@chromium.org, ... | 4 years, 11 months ago (2016-01-22 01:56:00 UTC)#7 |
|---|
| Dan Ehrenberg | littledan@chromium.org changed reviewers: + littledan@chromium.org | 4 years, 11 months ago (2016-01-22 01:56:30 UTC)#8 |
|---|
| Dan Ehrenberg | The CQ bit was checked by littledan@chromium.org | 4 years, 11 months ago (2016-01-22 01:56:30 UTC)#9 |
|---|
| Dan Ehrenberg | lgtm This change looks good to me. For the heap, I listed hpayer as TBR--this ... | 4 years, 11 months ago (2016-01-22 01:56:30 UTC)#10 |
|---|
| Dan Ehrenberg | The patchset sent to the CQ was uploaded after l-g-t-m from rossberg@chromium.org Link to the ... | 4 years, 11 months ago (2016-01-22 01:56:30 UTC)#11 |
|---|
| commit-bot: I haz the power | CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1581033002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1581033002/60001 | 4 years, 11 months ago (2016-01-22 01:56:37 UTC)#12 |
|---|
| commit-bot: I haz the power | The CQ bit was unchecked by commit-bot@chromium.org | 4 years, 11 months ago (2016-01-22 01:58:19 UTC)#13 |
|---|
| commit-bot: I haz the power | Try jobs failed on following builders: v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8\_android\_arm\_compile\_rel/builds/12378) v8_linux_arm_rel on tryserver.v8 (JOB_FAILED, ... | 4 years, 11 months ago (2016-01-22 01:58:20 UTC)#14 |
|---|
| caitp (gmail) | On 2016/01/22 01:58:20, commit-bot: I haz the power wrote: > Try jobs failed on following ... | 4 years, 11 months ago (2016-01-22 02:28:44 UTC)#15 |
|---|
| caitp (gmail) | The CQ bit was checked by caitpotter88@gmail.com | 4 years, 11 months ago (2016-01-22 02:34:27 UTC)#16 |
|---|
| caitp (gmail) | The patchset sent to the CQ was uploaded after l-g-t-m from rossberg@chromium.org, littledan@chromium.org Link to ... | 4 years, 11 months ago (2016-01-22 02:34:28 UTC)#17 |
|---|
| commit-bot: I haz the power | CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1581033002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1581033002/80001 | 4 years, 11 months ago (2016-01-22 02:34:31 UTC)#18 |
|---|
| commit-bot: I haz the power | The CQ bit was unchecked by commit-bot@chromium.org | 4 years, 11 months ago (2016-01-22 02:35:52 UTC)#19 |
|---|
| commit-bot: I haz the power | Try jobs failed on following builders: v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8\_linux64\_avx2\_rel/builds/9051) v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED, ... | 4 years, 11 months ago (2016-01-22 02:35:53 UTC)#20 |
|---|
| caitp (gmail) | The CQ bit was unchecked by caitpotter88@gmail.com | 4 years, 11 months ago (2016-01-22 02:36:21 UTC)#21 |
|---|
| caitp (gmail) | The CQ bit was checked by caitpotter88@gmail.com | 4 years, 11 months ago (2016-01-22 02:45:38 UTC)#22 |
|---|
| caitp (gmail) | The patchset sent to the CQ was uploaded after l-g-t-m from rossberg@chromium.org, littledan@chromium.org Link to ... | 4 years, 11 months ago (2016-01-22 02:45:38 UTC)#23 |
|---|
| commit-bot: I haz the power | CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1581033002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1581033002/100001 | 4 years, 11 months ago (2016-01-22 02:45:42 UTC)#24 |
|---|
| commit-bot: I haz the power | Committed patchset #6 (id:100001) | 4 years, 11 months ago (2016-01-22 03:12:37 UTC)#25 |
|---|
| commit-bot: I haz the power | Description was changed from ========== [es7] implement Object.values() / Object.entries() proposal BUG=v8:4663 LOG=N TBR=hpayer@chromium.org R=ljharb@gmail.com, ... | 4 years, 11 months ago (2016-01-22 03:13:07 UTC)#26 |
|---|
| commit-bot: I haz the power | Patchset 6 (id:??) landed as https://crrev.com/677be73e767f93741204f07061f0216485086f99 Cr-Commit-Position: refs/heads/master@{#33450} | 4 years, 11 months ago (2016-01-22 03:13:08 UTC)#27 |
|---|