Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29697596: Issue 6388 - Wrap inherited function properties as well (Closed)

Created:
Feb. 14, 2018, 2:46 p.m. by kzar
Modified:
Feb. 16, 2018, 5:08 p.m.
CC:
Sebastian Noack
Visibility:
Public.

Description

Issue 6388 - Wrap inherited function properties as well

Patch Set 1 #

Total comments: 2

Patch Set 2 : Simplify our approach #

Total comments: 8

Patch Set 3 : Use method shorthand #

Patch Set 4 : Improved comment #

Total comments: 1

Patch Set 5 : Final tweak to the comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -44 lines) Patch
M polyfill.js View 1 2 3 4 1 chunk +44 lines, -44 lines 0 comments Download

Messages

Total messages: 22
kzar
Patch Set 1
Feb. 14, 2018, 2:47 p.m. (2018-02-14 14:47:14 UTC) #1
Manish Jethani
On 2018/02/14 14:47:14, kzar wrote: > Patch Set 1 I'm not seeing the connection between ...
Feb. 14, 2018, 5:17 p.m. (2018-02-14 17:17:23 UTC) #2
kzar
On 2018/02/14 17:17:23, Manish Jethani wrote: > I'm not seeing the connection between this and ...
Feb. 14, 2018, 6:12 p.m. (2018-02-14 18:12:04 UTC) #3
a.giammarchi
Since I'm in CC, and if I might, this patch solves the ticket issue so, ...
Feb. 14, 2018, 6:57 p.m. (2018-02-14 18:57:21 UTC) #4
Manish Jethani
https://codereview.adblockplus.org/29697596/diff/29697597/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29697596/diff/29697597/polyfill.js#newcode87 polyfill.js:87: while (!descriptor && (objectProto = Object.getPrototypeOf(objectProto))); I think we ...
Feb. 15, 2018, 9:59 a.m. (2018-02-15 09:59:24 UTC) #5
Manish Jethani
On 2018/02/14 18:57:21, a.giammarchi wrote: > If it's an accessor, `get` and `set` are deleted ...
Feb. 15, 2018, 10:09 a.m. (2018-02-15 10:09:40 UTC) #6
a.giammarchi
On 2018/02/15 10:09:40, Manish Jethani wrote: > On 2018/02/14 18:57:21, a.giammarchi wrote: > > It ...
Feb. 15, 2018, 10:21 a.m. (2018-02-15 10:21:02 UTC) #7
Manish Jethani
On 2018/02/15 10:21:02, a.giammarchi wrote: > On 2018/02/15 10:09:40, Manish Jethani wrote: > > On ...
Feb. 15, 2018, 11:19 a.m. (2018-02-15 11:19:43 UTC) #8
a.giammarchi
On 2018/02/15 11:19:43, Manish Jethani wrote: > > So that's a bit out of idealism. ...
Feb. 15, 2018, 11:33 a.m. (2018-02-15 11:33:57 UTC) #9
Manish Jethani
On 2018/02/15 11:33:57, a.giammarchi wrote: > [snip] > > TL;DR if configurable, which is our ...
Feb. 15, 2018, 1:05 p.m. (2018-02-15 13:05:13 UTC) #10
Manish Jethani
https://codereview.adblockplus.org/29697596/diff/29697597/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29697596/diff/29697597/polyfill.js#newcode83 polyfill.js:83: do By the way I find this nicer too: ...
Feb. 15, 2018, 1:17 p.m. (2018-02-15 13:17:51 UTC) #11
a.giammarchi
On 2018/02/15 13:05:13, Manish Jethani wrote: > On 2018/02/15 11:33:57, a.giammarchi wrote: > > in ...
Feb. 15, 2018, 1:25 p.m. (2018-02-15 13:25:08 UTC) #12
Manish Jethani
On 2018/02/15 13:25:08, a.giammarchi wrote: > [snip] > > Accordingly, once we have `func` trapped, ...
Feb. 15, 2018, 2:31 p.m. (2018-02-15 14:31:11 UTC) #13
a.giammarchi
On 2018/02/15 14:31:11, Manish Jethani wrote: > On 2018/02/15 13:25:08, a.giammarchi wrote: > > The ...
Feb. 15, 2018, 2:57 p.m. (2018-02-15 14:57:04 UTC) #14
kzar
Patch Set 2: Simplify our approach I also did not know Object.defineProperty preserved the attributes ...
Feb. 16, 2018, 1:36 p.m. (2018-02-16 13:36:46 UTC) #15
a.giammarchi
LGTM ( the "last but best" one 😂 )
Feb. 16, 2018, 1:45 p.m. (2018-02-16 13:45:21 UTC) #16
Manish Jethani
https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js#newcode83 polyfill.js:83: // inherited its other attributes (e.g. enumerable) are presereved, ...
Feb. 16, 2018, 2:03 p.m. (2018-02-16 14:03:53 UTC) #17
Manish Jethani
On 2018/02/16 14:03:53, Manish Jethani wrote: > https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js > File polyfill.js (right): > > https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js#newcode83 ...
Feb. 16, 2018, 4:28 p.m. (2018-02-16 16:28:17 UTC) #18
kzar
Patch Set 3 : Use method shorthand Patch Set 4 : Improved comment https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js File ...
Feb. 16, 2018, 4:41 p.m. (2018-02-16 16:41:08 UTC) #19
Manish Jethani
https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29697596/diff/29699555/polyfill.js#newcode84 polyfill.js:84: // except accessor properties since we're specifying a value. ...
Feb. 16, 2018, 4:47 p.m. (2018-02-16 16:47:38 UTC) #20
Manish Jethani
https://codereview.adblockplus.org/29697596/diff/29699564/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29697596/diff/29699564/polyfill.js#newcode84 polyfill.js:84: // except accessor property attributes (e.g. get and set) ...
Feb. 16, 2018, 4:49 p.m. (2018-02-16 16:49:51 UTC) #21
kzar
Feb. 16, 2018, 5:08 p.m. (2018-02-16 17:08:37 UTC) #22
Thanks to you both, I'm happy with this now.

Powered by Google App Engine
This is Rietveld