[Normative] Add Object Rest Spread #1048

keithamus · 2017-12-19T00:30:06Z

Proposal repo: https://github.com/tc39/proposal-object-rest-spread/

keithamus · 2017-12-19T00:32:59Z

spec.html


        ArrayAssignmentPattern[Yield, Await] :
          `[` Elision? AssignmentRestElement[?Yield, ?Await]? `]`
          `[` AssignmentElementList[?Yield, ?Await] `]`
          `[` AssignmentElementList[?Yield, ?Await] `,` Elision? AssignmentRestElement[?Yield, ?Await]? `]`

+        AssignmentRestProperty[Yield, Await] :
+          `...` DestructuringAssignmentTarget[?Yield, ?Await]


In the spec text on the repo this AssignmentPropertyList production has [Yield, Await] but I'm sure it's meant to be [?Yield, ?Await]. I've made it [?Yield, ?Await] here to expedite things, but otherwise I'm happy to raise an issue in the repo and resolve it there before moving forward with this PR.

Changing it here as you've done seems completely appropriate to me. Wouldn't hurt to make a matching PR there, but it's also not essential to do.

littledan

Great work on this patch! Some comments, mostly editorial; there are a couple apparent typos. It's always hard to avoid errors here when you can't run the spec.

Minor: We generally use Normative: rather than [Normative] to describe the patch type.

littledan · 2017-12-22T12:16:47Z

spec.html

@@ -4764,6 +4764,35 @@ <h1>GetFunctionRealm ( _obj_ )</h1>
        <p>Step 5 will only be reached if _obj_ is a non-standard function exotic object that does not have a [[Realm]] internal slot.</p>
      </emu-note>
    </emu-clause>
+
+    <!-- es6num="7.3.23" -->


es6num should not be placed on new things; it's only used for cross-referencing for things in ES2015.

littledan · 2017-12-22T12:25:19Z

spec.html

+          1. ReturnIfAbrupt(_fromValue_).
+          1. Let _excludedNames_ be a new empty List.
+          1. Return ? CopyDataProperties(_object_, _fromValue_, _excludedNames_).
+        </emu-alg>


Minor: I'd prefer if this new clause is added at the end of the list of productions, making it parallel with how the syntax is defined.

littledan · 2017-12-22T12:26:13Z

spec.html


        ArrayAssignmentPattern[Yield, Await] :
          `[` Elision? AssignmentRestElement[?Yield, ?Await]? `]`
          `[` AssignmentElementList[?Yield, ?Await] `]`
          `[` AssignmentElementList[?Yield, ?Await] `,` Elision? AssignmentRestElement[?Yield, ?Await]? `]`

+        AssignmentRestProperty[Yield, Await] :
+          `...` DestructuringAssignmentTarget[?Yield, ?Await]


Changing it here as you've done seems completely appropriate to me. Wouldn't hurt to make a matching PR there, but it's also not essential to do.

littledan · 2017-12-22T12:27:28Z

spec.html

@@ -14306,14 +14344,18 @@ <h2>Supplemental Syntax</h2>

        ObjectAssignmentPattern[Yield, Await] :
          `{` `}`
+          `{` AssignmentRestProperty[?Yield, ?Await]? `}`


The ? at the end seems a little redundant with the { } in the previous line; I think you could choose one or the other. Looks like the proposal repository deletes line 14346; would that make sense to do here?

I've kept the `{` `}` line as this keeps the AssignmentRestProperty syntax in line with the others.

littledan · 2017-12-22T12:29:28Z

spec.html

@@ -14366,16 +14408,17 @@ <h1>Runtime Semantics: DestructuringAssignmentEvaluation</h1>
        <emu-grammar>ObjectAssignmentPattern : `{` `}`</emu-grammar>
        <emu-alg>
          1. Perform ? RequireObjectCoercible(_value_).
+          1. Perform ? PropertyDestructuringAssignmentEvaluation for |AssignmentPropertyList| using _value_ as the argument.


I'm not sure what |AssignmentPropertyList| refers to here. Are you sure you want to add this line?

littledan · 2017-12-22T12:30:05Z

spec.html

-          ObjectAssignmentPattern :
-            `{` AssignmentPropertyList `}`
-            `{` AssignmentPropertyList `,` `}`
+        <emu-grammar>ObjectAssignmentPattern :


Minor: Was deleting the newline here deliberate?

littledan · 2017-12-22T13:10:17Z

spec.html

+        <h1>Runtime Semantics: PropertyDestructuringAssignmentEvaluation</h1>
+        <p>With parameters _value_.</p>
+
+        <emu-note>These collect a list of all destructured property names rather than just empty completion.</emu-note>


The "rather than" part of the note might not be necessary anymore, as it describes what changed from one version of the spec to the other.

keithamus · 2017-12-22T22:10:06Z

@littledan I believe I've addressed all of your comments. Let me know if you have any more!

littledan

The fixed seem good AFAICT. Saw two more small issues I missed before.

littledan · 2017-12-22T23:29:02Z

spec.html

        <emu-grammar>AssignmentProperty : IdentifierReference Initializer?</emu-grammar>
        <emu-alg>
          1. Let _P_ be StringValue of |IdentifierReference|.
          1. Let _lref_ be ? ResolveBinding(_P_).
          1. Let _v_ be ? GetV(_value_, _P_).
          1. If |Initializer_opt| is present and _v_ is *undefined*, then
            1. Let _defaultValue_ be the result of evaluating |Initializer|.
-            1. Set _v_ to ? GetValue(_defaultValue_).
+            1. Let _v_ be ? GetValue(_defaultValue_).


I'm not sure why you made this change.

I find this change confusing since it is unclear if this shadows or mutates this binding in the conditional.

littledan · 2017-12-22T23:32:37Z

spec.html

@@ -15463,14 +15557,18 @@ <h2>Syntax</h2>

        ObjectBindingPattern[Yield, Await] :
          `{` `}`
+          `{` BindingRestProperty[?Yield, ?Await]? `}`


These two lines are ambiguous; it would be fine to either remove the ? Or delete the first of the two lines.

sebmarkbage

Looks good to me other than the mentioned nits.

sebmarkbage · 2017-12-22T23:53:50Z

spec.html

        <emu-grammar>AssignmentProperty : IdentifierReference Initializer?</emu-grammar>
        <emu-alg>
          1. Let _P_ be StringValue of |IdentifierReference|.
          1. Let _lref_ be ? ResolveBinding(_P_).
          1. Let _v_ be ? GetV(_value_, _P_).
          1. If |Initializer_opt| is present and _v_ is *undefined*, then
            1. Let _defaultValue_ be the result of evaluating |Initializer|.
-            1. Set _v_ to ? GetValue(_defaultValue_).
+            1. Let _v_ be ? GetValue(_defaultValue_).


I find this change confusing since it is unclear if this shadows or mutates this binding in the conditional.

keithamus · 2017-12-23T19:53:35Z

All nits addressed 😄

ljharb · 2017-12-23T21:12:57Z

spec.html

+        <emu-alg>
+          1. Let _exprValue_ be the result of evaluating _AssignmentExpression_.
+          1. Let _fromValue_ be GetValue(_exprValue_).
+          1. ReturnIfAbrupt(_fromValue_).


This line should be removed, and the preceding GetValue be prefixed with a ?

ljharb · 2017-12-23T21:13:18Z

spec.html

+          1. ReturnIfAbrupt(_lref_).
+          1. Let _restObj_ be ObjectCreate(%ObjectPrototype%).
+          1. Let _assignStatus_ be CopyDataProperties(_restObj_, _value_, _excludedNames_).
+          1. ReturnIfAbrupt(_assignStatus_).


This line should be removed, and the preceding CopyDataProperties be prefixed with a ?

ljharb · 2017-12-23T21:13:35Z

spec.html

+        <emu-alg>
+          1. Let _restObj_ be ObjectCreate(%ObjectPrototype%).
+          1. Let _assignStatus_ be CopyDataProperties(_restObj_, _value_, _excludedNames_).
+          1. ReturnIfAbrupt(_assignStatus_).


This line should be removed, and the preceding CopyDataProperties be prefixed with a ?

ljharb · 2017-12-23T21:13:43Z

spec.html

+          1. ReturnIfAbrupt(_assignStatus_).
+          1. Let _bindingId_ be StringValue of |BindingIdentifier|.
+          1. Let _lhs_ be ResolveBinding(_bindingId_, _environment_).
+          1. ReturnIfAbrupt(_lhs_).


This line should be removed, and the preceding ResolveBinding be prefixed with a ?

keithamus · 2017-12-24T15:36:18Z

Done and done

littledan

Eesh, sorry, noticed these additional nits. Anyway, LGTM modulo having at least the first one (about Type) fixed.

littledan · 2017-12-25T11:03:12Z

spec.html

+      <p>When the abstract operation CopyDataProperties is called with arguments _target_, _source_ and _excluded_, the following steps are taken:</p>
+      <emu-alg>
+        1. Assert: Type(_target_) is Object.
+        1. Assert: Type(_excluded_) is List.


_excluded_ is not an ECMAScript object and so it doesn't have a type. A better wording is, Assert: _excluded_ is a List of property keys.

_excluded_ is not an ECMAScript object and so it doesn't have a type.

Not so: the definition of Type(x) specifically allows specification types as well as ECMAScript language types. However, it's true that this capability isn't used much, and never to identify Lists, so I agree with the suggested change.

Oh, my mistake! Thanks for the background, but sounds like it's still a reasonable change.

littledan · 2017-12-25T11:04:29Z

spec.html

+        1. Else,
+          1. Let _from_ be ! ToObject(_source_).
+          1. Let _keys_ be ? _from_.[[OwnPropertyKeys]]().
+        1. Repeat for each element _nextKey_ of _keys_ in List order,


The wording of spec language has been getting regularized and changed a little since this proposal was written. The modern wording is, For each element _nextKey_ of _keys_, do. For an example, see the current definition of Object.assign.

It's also okay to say For each element _nextKey_ of _keys_ in List order, do.

littledan · 2017-12-25T11:05:44Z

spec.html

-            `{` AssignmentPropertyList `}`
-            `{` AssignmentPropertyList `,` `}`
+          `{` AssignmentPropertyList `}`
+          `{` AssignmentPropertyList `,` `}`


What was the reason for removing this indentation?

littledan · 2017-12-25T11:09:21Z

spec.html

        <emu-grammar>BindingProperty : SingleNameBinding</emu-grammar>
        <emu-alg>
          1. Let _name_ be the string that is the only element of BoundNames of |SingleNameBinding|.
-          1. Return the result of performing KeyedBindingInitialization for |SingleNameBinding| using _value_, _environment_, and _name_ as the arguments.
+          1. Let _status_ be the result of performing KeyedBindingInitialization for |SingleNameBinding| using _value_, _environment_, and _name_ as the arguments.
+          1. ReturnIfAbrupt(_status_).


If you're doing these conversions to ? syntax, you might be able to apply it here too, and use Perform ? KeyBindingInitialization for |SingleNameBinding| using _value_, _environment_, and _name_ as the arguments. There are some other uses of ReturnIfAbrupt in this PR that you could remove similarly. However, the current spec seems to have some uses of ? and some ReturnIfAbrupt in a way that I don't really understand; maybe these uses are not allowed for some reason.

I've changed any ReturnIfAbrupt where the Let value isn't used. In other words, now the only usage of ReturnIfAbrupt is where the Let is used later in the steps.

littledan · 2017-12-25T11:11:40Z

spec.html

+          1. Let _keys_ be ? _from_.[[OwnPropertyKeys]]().
+        1. Repeat for each element _nextKey_ of _keys_ in List order,
+          1. Let _found_ be *false*.
+          1. Repeat for each element _e_ of _excluded_,


Ditto the new wording here.

keithamus · 2018-01-03T11:08:00Z

That should be all comments addressed again. cc @ljharb @littledan @bterlson

ljharb · 2018-01-03T16:48:27Z

spec.html

+      <emu-alg>
+        1. Assert: Type(_target_) is Object.
+        1. Assert: _excluded_ is a List of property keys.
+        1. If _source_ is *undefined* or *null*, let _keys_ be a new empty List.


Would it be cleaner to return target directly from here, instead of making an empty list and implicitly skipping the keys iteration?

ljharb · 2018-01-03T16:51:58Z

spec.html

+          1. Let _lref_ be the result of evaluating |DestructuringAssignmentTarget|.
+          1. ReturnIfAbrupt(_lref_).
+          1. Let _restObj_ be ObjectCreate(%ObjectPrototype%).
+          1. Let _assignStatus_ be ? CopyDataProperties(_restObj_, _value_, _excludedNames_).


assignStatus is unused here; this should probably be Perform ? CopyDataProperties…

ljharb · 2018-01-03T16:54:27Z

spec.html

+        <emu-grammar>BindingRestProperty : `...` BindingIdentifier</emu-grammar>
+        <emu-alg>
+          1. Let _restObj_ be ObjectCreate(%ObjectPrototype%).
+          1. Let _assignStatus_ be ? CopyDataProperties(_restObj_, _value_, _excludedNames_).


Also Perform here

anba · 2018-01-04T18:19:06Z

I guess the following issues still apply to the current version and therefore should be fixed before the PR gets merged.

tc39/proposal-object-rest-spread/issues/44
tc39/proposal-object-rest-spread/issues/42
tc39/proposal-object-rest-spread/issues/36

keithamus · 2018-01-04T22:31:06Z

spec.html

+        1. For each element _nextKey_ of _keys_ in List order, do
+          1. Let _found_ be *false*.
+          1. For each element _e_ of _excluded_ in List order, do
+            1. If SameValue(_e_, _nextKey_) is true, then


Hopefully this satisfies tc39/proposal-object-rest-spread#44 @anba

keithamus · 2018-01-04T22:31:28Z

spec.html

+        <emu-grammar>PropertyDefinition : `...` AssignmentExpression</emu-grammar>
+        <emu-alg>
+          1. Return ~empty~.
+        </emu-alg>


Hopefully this satisfies tc39/proposal-object-rest-spread#42 @anba

keithamus · 2018-01-04T22:32:23Z

spec.html

+
+        <emu-grammar>BindingRestProperty : `...` BindingIdentifier</emu-grammar>
+        <emu-alg>
+          1. Let _lhs_ be ? ResolveBinding(StringValue of |BindingIdentifier|, _environment_).


Hopefully this satisfies tc39/proposal-object-rest-spread#36 @anba

bterlson · 2018-01-11T21:38:15Z

spec.html

+          1. Let _from_ be ! ToObject(_source_).
+          1. Let _keys_ be ? _from_.[[OwnPropertyKeys]]().
+        1. For each element _nextKey_ of _keys_ in List order, do
+          1. Let _found_ be *false*.


I think _found_ should be _isExcluded_ or similar (little more clear what it's used for).

Actually you could consider just excluded for the boolean, and use excludedNames for the list like you do further down.

bterlson · 2018-01-11T21:40:39Z

spec.html

+
+      <emu-clause id="Rest-RuntimeSemantics-PropertyDestructuringAssignmentEvaluation">
+        <h1>Runtime Semantics: PropertyDestructuringAssignmentEvaluation</h1>
+        <p>With parameters _value_.</p>


Singular parameter here

bterlson · 2018-01-11T21:47:13Z

spec.html

+        <h1>Runtime Semantics: PropertyDestructuringAssignmentEvaluation</h1>
+        <p>With parameters _value_.</p>
+
+        <emu-note>These collect a list of all destructured property names.</emu-note>


This note isn't very descriptive to my eye. these could be "The following operations" or "the operations in this clause". Also, can we say that collecting a list is in addition to performing evaluation semantics? This could probably just be folded into the normative clause intro prose.

bterlson · 2018-01-11T22:31:22Z

I think this is great! Other than the minor comments previously, the only other issue here is the section IDs. It'll be annoying to find and replace but I'd like them to be consistent with the style used in 262 (kebab case prefixed with sec-)

anba · 2018-01-11T23:26:57Z

spec.html

+        1. Assert: Type(_target_) is Object.
+        1. Assert: _excluded_ is a List of property keys.
+        1. If _source_ is *undefined* or *null*, return _target_.
+        1. Else,


I think I'd prefer if this "Else" was just dropped because the if-branch always returns.

anba · 2018-01-11T23:27:32Z

spec.html

+
+    <emu-clause id="sec-copydataproperties" aoid="CopyDataProperties">
+      <h1>CopyDataProperties (_target_, _source_, _excluded_)</h1>
+      <p>When the abstract operation CopyDataProperties is called with arguments _target_, _source_ and _excluded_, the following steps are taken:</p>


Comma before "and"

That is a style choice I think 🤔

Is there an english grammar/style guide for ECMA?

pretty sure we do use the Oxford comma, so definitely this needs to be fixed

anba · 2018-01-11T23:30:11Z

spec.html

+        1. Return _target_.
+      </emu-alg>
+      <emu-note>
+        <p>The target passed is in here is always a newly created object which doesn't leak in case of an error being thrown. It's not observable when the actual object construction and initialization happens nor whether it is interleaved with reading the properties off the original object.</p>


s/is in/in/

"leak" is probably a bit too vague. The only time "leak" is used in the current spec is in the context of a resource leak. For CopyDataProperties we should instead use something along the lines of "directly accessible" wording used elsewhere in the current spec.

The second sentence could also easily be misinterpreted, because reading "properties off [sic] the original object" can't be interleaved with property evaluation of other properties in the object literal. Maybe just drop the second sentence?

anba · 2018-01-11T23:47:28Z

spec.html

@@ -11848,6 +11881,7 @@ <h1>Runtime Semantics: PropertyDefinitionEvaluation</h1>
          1. Perform ? PropertyDefinitionEvaluation of |PropertyDefinitionList| with arguments _object_ and _enumerable_.
          1. Return the result of performing PropertyDefinitionEvaluation of |PropertyDefinition| with arguments _object_ and _enumerable_.
        </emu-alg>
+        <emu-grammar>PropertyDefinition : `...` AssignmentExpression</emu-grammar>


This should be next to the corresponding <emu-alg>.

anba · 2018-01-11T23:48:41Z

spec.html

@@ -11868,6 +11902,12 @@ <h1>Runtime Semantics: PropertyDefinitionEvaluation</h1>
          1. Assert: _enumerable_ is *true*.
          1. Return CreateDataPropertyOrThrow(_object_, _propKey_, _propValue_).
        </emu-alg>
+        <emu-alg>


Belongs after the following <emu-note>

anba · 2018-01-12T00:07:02Z

spec.html

+        <h1>Runtime Semantics: RestDestructuringAssignmentEvaluation</h1>
+        <p>With parameters _value_ and _excludedNames_.</p>
+
+        <emu-grammar>AssignmentRestProperty[Yield, Await] : `...` DestructuringAssignmentTarget</emu-grammar>


No [Yield, Await] here.

anba · 2018-01-12T00:08:32Z

spec.html

+        <emu-alg>
+          1. Let _excludedNames_ be the result of performing PropertyBindingInitialization of |BindingPropertyList| using _value_ and _environment_ as arguments.
+          1. ReturnIfAbrupt(_excludedNames_).
+          1. Return the result of performing RestBindingInitialization of _BindingRestProperty_ with _value_, _environment_ and _excludedNames_ as the arguments.


Comma before "and"

anba · 2018-01-12T00:08:45Z

spec.html

+        <emu-grammar>ObjectBindingPattern : `{` BindingRestProperty `}`</emu-grammar>
+        <emu-alg>
+          1. Let _excludedNames_ be a new empty List.
+          1. Return the result of performing RestBindingInitialization of _BindingRestProperty_ with _value_, _environment_ and _excludedNames_ as the arguments.


Comma before "and".

anba · 2018-01-12T00:11:05Z

spec.html

+        </emu-alg>
+      </emu-clause>
+
+      <emu-clause id="Rest-RuntimeSemantics-PropertyBindingInitialization">


As for destructuring assignment evaluation, I'd keep these semantics in the current <emu-clause>.

Could you clarify here please?

I think he's suggesting that you can just add the new production under the existing clause rather than adding a new clause for it.

anba · 2018-01-12T00:15:45Z

spec.html

+
+      <emu-clause id="Rest-RuntimeSemantics-RestBindingInitialization">
+        <h1>Runtime Semantics: RestBindingInitialization</h1>
+        <p>With parameters _value_, _environment_ and _excludedNames_.</p>


Comma before "and".

keithamus · 2018-01-22T13:43:21Z

All notes (bar #1048 (comment)) have been addressed.

styfle · 2018-01-29T14:51:33Z

@sebmarkbage @ljharb Should this be merged?

I see it already moved to Stage 4.

tc39/proposals@7a04292#diff-79a53ca5c00fe0d90f18cd5237280da3

tc39/proposal-object-rest-spread#32

ljharb · 2018-01-29T16:33:46Z

@styfle it will be merged when the editor is ready to do it :-) thanks for your patience!

Proposal repo: https://github.com/tc39/proposal-object-rest-spread/

…ntics

ljharb requested review from bterlson and sebmarkbage December 19, 2017 00:32

keithamus commented Dec 19, 2017

View reviewed changes

ljharb added the pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. label Dec 19, 2017

chicoxyzzy mentioned this pull request Dec 19, 2017

stage-4. what remains from the list of "Entrance Criteria" list? tc39/proposal-object-rest-spread#32

Closed

5 tasks

littledan reviewed Dec 22, 2017

View reviewed changes

keithamus force-pushed the add-object-rest-spread branch from 6b57494 to cc2fa78 Compare December 22, 2017 22:03

keithamus mentioned this pull request Dec 22, 2017

fix typo in AssignmentRestProperty DestructuringAssignmentTarget tc39/proposal-object-rest-spread#58

Merged

littledan reviewed Dec 22, 2017

View reviewed changes

sebmarkbage reviewed Dec 23, 2017

View reviewed changes

keithamus force-pushed the add-object-rest-spread branch from cc2fa78 to 73cd0e8 Compare December 23, 2017 19:52

ljharb requested changes Dec 23, 2017

View reviewed changes

keithamus force-pushed the add-object-rest-spread branch from 73cd0e8 to 854dd18 Compare December 24, 2017 15:36

littledan approved these changes Dec 25, 2017

View reviewed changes

keithamus force-pushed the add-object-rest-spread branch from 854dd18 to 057a36c Compare January 3, 2018 11:04

ljharb reviewed Jan 3, 2018

View reviewed changes

keithamus force-pushed the add-object-rest-spread branch from 057a36c to 432b866 Compare January 4, 2018 22:30

keithamus commented Jan 4, 2018

View reviewed changes

keithamus mentioned this pull request Jan 7, 2018

Why is this useful grammer still in stage 3? tc39/proposal-object-rest-spread#60

Closed

bterlson reviewed Jan 11, 2018

View reviewed changes

anba reviewed Jan 12, 2018

View reviewed changes

keithamus force-pushed the add-object-rest-spread branch from 432b866 to 8e36950 Compare January 22, 2018 13:43

bterlson approved these changes Jan 23, 2018

View reviewed changes

ljharb approved these changes Jan 23, 2018

View reviewed changes

styfle mentioned this pull request Jan 24, 2018

Implement Object Rest & Spread chakra-core/ChakraCore#3406

Closed

ljharb assigned bterlson Jan 29, 2018

bterlson added has consensus This has committee consensus. has test262 tests and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels Feb 7, 2018

jmdyck mentioned this pull request Feb 9, 2018

Normative: provide default prototype and realm values for CreateBuiltinFunction #1044

Merged

keithamus and others added 4 commits February 12, 2018 16:16

Normative: Add Object Rest Spread

83e64c5

Proposal repo: https://github.com/tc39/proposal-object-rest-spread/

Editorial: move new static semantics clause into existing static sema…

c74edb6

…ntics

Editorial: Fix up ids

293742b

Editorial: missing comma

705157d

bterlson force-pushed the add-object-rest-spread branch from 8e36950 to 705157d Compare February 13, 2018 00:18

bterlson merged commit 17ebeea into tc39:master Feb 13, 2018

styfle mentioned this pull request Feb 13, 2018

Move Stage 3 to Stage 4 in README tc39/proposal-object-rest-spread#62

Merged

keithamus deleted the add-object-rest-spread branch February 13, 2018 22:13

Jessidhia mentioned this pull request Apr 18, 2018

WIP: rename proposal-object-rest-spread to transform-object-rest-spread babel/babel#7406

Closed

caitp mentioned this pull request Jul 6, 2018

Initial set for Object Spread feature tc39/test262#890

Merged

[Normative] Add Object Rest Spread #1048

[Normative] Add Object Rest Spread #1048

Conversation

keithamus commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithamus commented Dec 22, 2017

littledan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebmarkbage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithamus commented Dec 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithamus commented Dec 24, 2017

littledan left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithamus commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anba commented Jan 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bterlson commented Jan 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithamus commented Jan 22, 2018

styfle commented Jan 29, 2018

ljharb commented Jan 29, 2018

littledan left a comment •

edited