Allow extended attributes to apply to types #286

domenic · 2017-01-25T23:06:19Z

This implements the plan discussed in https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0003.html and sets the stage for using extended attributes for SharedArrayBuffer-related purposes.

@bzbarsky, I could use some close review on this. Some points for discussion:

Is [Clamp] long being a distinct type from long a good way of doing this? I based that on how long? is different from long, and it seems like a good mental model.
Is it sufficiently clear that extended attributes propagate through typedefs? (A point you called out specifically in your email.) If not, any suggestions on how to make this clear?
We have separate TypeWithAttributes and Type mainly so that ReturnType does not get attributes. Does that seem good?
I didn't allow (gramatically) specifying an extended attribute over an entire union. I forget why, but an hour ago it seemed like a bad idea... if you have any thoughts to help guide me one way or another on this that would be appreciated.
This will invalidate a lot of existing IDL which will need to get updated. E.g. [TreatNullAs=EmptyString] attribute DOMString data; must become attribute [TreatNullAs=EmptyString] DOMString data;. I think this is the right way to go, as it's clearer how things work and doesn't require any propagation of extended attributes from attribute/argument declarations into their types, which sounds annoying to spec and would cause inconsistency as sometimes people would do that and sometimes not. I also think this would be a good test case for future more-disruptive changes.
TreatNullAs questions:
- Searching Chromium I find no instances of this being used on callback interfaces. Can we nuke that part of the spec?
- While we're here, since this is going to invalidate existing IDL anyway, can we just get rid of the silly identifier argument and make this [TreatNullAsEmptyString]?

I hope I didn't miss anything major; this seemed almost too easy...

Preview | Diff

bzbarsky · 2017-01-26T04:55:59Z

I'm pretty swamped with reviews for various stuff right now, so it will likely take me a few days to get to this. I'm sorry. I simply have a huge backlog, both in terms of specs and in terms of code. :(

You should probably look at https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0009.html in case it affects your approach here.

One question I do have up front: what is the behavior you're actually aiming for with the patches applied? Knowing that will let me evaluate the behavior, as well as whether the actual spec changes implement that behavior correctly. Ideally, that sort of thing would live in the commit message... I ask this because you say you're implementing the plan discussed in https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0003.html but I actually outlined a few different options in that mail, and I'm not sure which ones you decided to take.

Some responses to questions without having read the changes very carefully yet.

Is [Clamp] long being a distinct type from long a good way of doing this?

That seems reasonable, at first glance, yes.

Will take some thought to check whether expressing interactions like "not both [Clamp] and [EnforceRange]" works right in that setup.

Is it sufficiently clear that extended attributes propagate through typedefs?

I don't think it is. In particular, it's not obvious to me how they propagate, exactly, based on searching the diff for "typedef". Like what the actual result is. I don't quite recall what model the spec has of typedefs in general, nor what implementations do, exactly... :(

We have separate TypeWithAttributes and Type

That's fine, I think. ReturnType definitely shouldn't have attributes. Then again, ReturnType is already separate from Type, so we don't really care about this part.

There are other places where types probably shouldn't have attributes (at least not the attributes we're defining). For example, callback argument types, right?

I didn't allow (gramatically) specifying an extended attribute over an entire union.

I'm not sure what you mean. Your grammar has "ExtendedAttributeList UnionType Null" as a possible thing for TypeWithAttributes, right?

This will invalidate a lot of existing IDL which will need to get updated.

Hmm. So we could support the old syntax, in theory, for attributes. As you note, it would be complicated.

In practice, TreatNullAs is ony used in a few specs: HTML, CSSOM, DOM. Updating them would not be a huge deal. I see ~40 places in Gecko's IDL using this annotation, which is not so bad to just update en-masse.

Can we nuke that part of the spec?

I think so. I see no consumers in Gecko either, and we really shouldn't add new ones, imo.

can we just get rid of the silly identifier argument

That's probably fine, yes.

domenic · 2017-01-26T05:14:00Z

No problem on the delay.

One question I do have up front: what is the behavior you're actually aiming for with the patches applied.

(Yes, the commit message needs work.)

The basic idea is that the three extended attributes [Clamp], [ExtendRange], and [TreatNullAs] can now only be used on types, instead of in their previous positions as annotations on method arguments, attributes, and dictionary members. And they should only be usable in positions where they affect conversions from ES to Web IDL. (Hmm, now that I put it that way, maybe we should grammatically disallow them on non-readonly attributes.)

This adds the new concept of "types annotated with extended attributes". That is, [Clamp] long is a new type, consisting of long "annotated with" [Clamp].

In the spec, we say that the sections on the types like long also govern versions generated by extended attributes, and then the algorithm for converting ES to long says "if the type is annotated with [Clamp]".

I couldn't really find a formal model for "is annotated with"; it's used elsewhere without any real precision. (E.g., there is no set of annotations on a given grammar production or construct that we consult. I guess this is what leads to problems like "what happens if you specify [SecureContext] on a mixin or partial interface?") Maybe I could achieve something more precise by studying the nullable types more.

Will take some thought to check whether expressing interactions like "not both [Clamp] and [EnforceRange]" works right in that setup.

This again hinges on the "annotated with" concept. We just say "a type cannot be annotated by both [Clamp] and [EnforceRange]". If we are comfortable with leaving the definition of "annotated with" vague, we're good to go, but are we? For example, it's not exactly clear how "annotated with" propagates through typedefs. Again maybe I can study how nullable propagates through typedefs.

That's fine, I think. ReturnType definitely shouldn't have attributes. Then again, ReturnType is already separate from Type, so we don't really care about this part.

What I meant is, I could have just changed Type, except then ReturnType would inherit the changes, and we don't want that. So that's where there are distinct grammar productions.

There are other places where types probably shouldn't have attributes (at least not the attributes we're defining). For example, callback argument types, right?

Good catch, thanks. Will fix.

I'm not sure what you mean. Your grammar has "ExtendedAttributeList UnionType Null" as a possible thing for TypeWithAttributes, right?

Oh, oops. I guess I meant I didn't allow specifying annotations on individual types inside a union, e.g. (Node or [Clamp] long). Probably I should...

Hmm. So we could support the old syntax, in theory, for attributes. As you note, it would be complicated.

In practice, TreatNullAs is ony used in a few specs: HTML, CSSOM, DOM. Updating them would not be a huge deal. I see ~40 places in Gecko's IDL using this annotation, which is not so bad to just update en-masse.

This is the same issue as https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0009.html, correct?

And yeah, I think we should bite the bullet. En-masse update for TreatNullAs, Clamp, and EnforceRange.

Can we nuke that part of the spec?

I think so. I see no consumers in Gecko either, and we really shouldn't add new ones, imo.

Great. Maybe I'll do that as a separate PR first.

bzbarsky · 2017-01-26T05:20:46Z

I guess I meant I didn't allow specifying annotations on individual types inside a union

Ah. It seems like we do want to allow that. On the other hand, specifying [Clamp] or [EnforceRange] or [TreatNullAs] on a union as a whole doesn't really make sense. But in terms of grammar, I don't see a great way to disallow it (because the union could be hidden behind a typedef, say)... We can just enforce it via the "can't be specified except on these types" validation mechanism.

This is the same issue as https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0009.html, correct?

I'm not sure what you mean. I mean, we do want to keep allowing extended attributes on attributes in general. And allow them on the type of the attribute. That's not a problem because the two locations (start of attribute, start of attribute type) are not adjacent, so the grammar is unambiguous. The only place where they are adjacent are non-required dictionary members and non-optional operation arguments...

domenic · 2017-01-26T05:24:38Z

I'm not sure what you mean. I mean, we do want to keep allowing extended attributes on attributes in general. And allow them on the type of the attribute. That's not a problem because the two locations (start of attribute, start of attribute type) are not adjacent, so the grammar is unambiguous. The only place where they are adjacent are non-required dictionary members and non-optional operation arguments...

Ah oops, I was indeed confusing two issues.

So the question is whether we have a use case for specifying extended attributes on operation arguments and dictionary members, instead of specifying them on their types. The spec does not, but Gecko does, and Gecko's reasons seem reasonably widely applicable. So I guess we should figure out how that works...

bzbarsky · 2017-01-26T05:28:26Z

I think the simplest way to spec it is that non-optional operation arguments have a Type, not a TypeWithAttributes. But for certain extended attributes, if they're specified on the operation argument they get applied to its type. Similar for non-required dictionary members.

It's a bit annoying on the spec side, but pretty clear in terms of actual usage.

tobie · 2017-01-30T12:16:49Z

index.bs

@@ -1480,7 +1477,7 @@ are applicable only to regular attributes:

 <pre class="grammar" id="prod-AttributeRest">
    AttributeRest :
-        "attribute" Type AttributeName ";"
+        "attribute" TypeWithAttributes AttributeName ";"


I agree TypeWithExtendedAttributes is long, but I think it would help make things clearer, here.

tobie · 2017-01-30T12:46:53Z

index.bs

+The [=type name=] of a type annotated with an [=extended attribute=] is
+the concatenation of the type name of the annotated type and a string
+corresponding to the extended attribute's identifier.
+


An example would be nice here.

I image what you want is ClampLong, not LongClamp. So the sentence above would imply that more clearly if it was reversed:

The [=type name=] of a type annotated with an [=extended attribute=] is the concatenation of the string corresponding to the extended attribute's identifier and the type name of the annotated type.

LongClamp seems fine.

Oh, you had LongClamp in mind. I see. I feel like that's counterintuitive, given the syntax is [Clamp] long.

But that's a minor issue, really.

In general I think it's not worth fussing over this or creating an example given that nobody knows what type names are used for.

Fwiw, Gecko uses type names to auto-generate union types and their accessors. So for every type in the union, there is an IsFoo() accessor on the union, where Foo is the type name.

It ends up being pretty ugly in practice. ;)

tobie · 2017-01-30T13:21:08Z

index.bs

-must not be specified on an operation argument,
-attribute or operation return value whose type is not {{DOMString}}.
+The [{{TreatNullAs}}] extended attribute must not be specified on a [=type=] that is not
+{{DOMString}}.


While you're at it, get rid of the double negation? E.g.:

The [{{TreatNullAs}}] extended attribute can only be specified on the {{DOMString}} [=type=].

"can" is not a normative word, so I don't think that really works.

domenic · 2017-02-03T23:34:14Z

OK, this is ready for another review. I think it meets our goals now.

annevk · 2017-02-07T10:27:13Z

Does this also mean we're going to change USVString back to [EnsureUTF16] DOMString as extended attributes are no longer problematic? Is there any other goal here other than avoiding doubling the number of typed-array-related types?

bzbarsky · 2017-02-07T15:02:51Z

Does this also mean we're going to change USVString back to [EnsureUTF16] DOMString

We could do that if we wanted to, yes. Whether we want to is less clear.

Is there any other goal here other than avoiding doubling the number of typed-array-related types?

Or quadrupling...

But note that this also fixes longstanding issues wherein you couldn't have a [Clamp] integer in a union or a dictionary, for example.

domenic · 2017-02-13T22:05:26Z

Ping @bzbarsky

dontcallmedom · 2017-03-21T16:47:37Z

for sake of book keeping, this would also address https://www.w3.org/Bugs/Public/show_bug.cgi?id=28834 (which is something that is also needed in w3c/mediacapture-main#384

tobie

As discussed over irc, I think we need to clarify the relationship and naming between the annotated type and it's "inner type" and figure out what incidence that has over the overload resolution algorithm.

Other than that and the few nits mentioned above, this is LGTM (and is pretty awesome :)).

Additionally, filed #334, because extended attribute usage as a whole lacks examples (but we shouldn't fix it here).

tobie · 2017-03-27T15:34:46Z

index.bs

+    The <dfn for="IDL type" lt="extended attribute associated with|extended attributes associated with">extended attributes associated with</dfn>
+    a [=type=] |type| are determined as follows:
+
+    1.  Let |extended attributes| be the empty set.


Link to infra ordered set?

tobie · 2017-03-27T15:51:44Z

index.bs

+        within an <emu-nt><a href="#prod-DictionaryMember">DictionaryMember</a></emu-nt> production,
+        add to |extended attributes| all of the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.


Shouldn't we have a (possibly non-exported) dfn for this?

tobie · 2017-03-27T15:53:12Z

index.bs

+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+    1.  If |type| is a [=typedef=], add the [=extended attributes associated with=] the type being
+        given a new name to |extended attributes|.


"the type being given a new name" probably also deserves its own dfn.

tobie · 2017-03-27T15:56:29Z

index.bs

+[=extended attributes=] that are
+<a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+
+The [=type name=] of a type associated with [=extended attributes=] is the concatenation of the


I know this doesn't matter a lot, but it would be nice to be a tad more explicit in which order this concatenation happens, e.g.:

[…] [=extended attribute associated with=] the type, in this order.

…and how it interacts with nullable types; i.e. for [Clamped] Foo? is it: FooClampedOrNull or FooOrNullClamped? It's the former imho, but this needs to be explicitly specified somewhere.

It is specified, since the type is Foo? and the new type is [Clamped] Foo?. I'll add an example though.

OK, I can't add an example since it's actually impossible to use any of the current type-applicable extended attributes with nullable types, or use them together.

tobie · 2017-03-27T15:56:42Z

index.bs

@@ -6099,6 +6129,7 @@ type is the concatenation of the type name for |T| and the string
 An <dfn id="dfn-extended-attribute" export>extended attribute</dfn> is an annotation
 that can appear on
 definitions,
+[=types=],


tobie · 2017-03-27T23:34:16Z

index.bs

+
+Additional types can be created from existing ones by specifying certain [=extended attributes=] on
+the existing types. For example, <code>[Clamp] long</code> defines a new type whose behavior is
+based on that of the {{long}} type, but modified as specified by the [{{Clamp}}] extended attribute.


Should we add a dfn to name those new types?

It seem you're using "the type annotated with the extended attribute" and "the extended attribute associated with type x".

As discussed over irc, nullables have the "inner" and "nullable type" concepts. We probably need something similar here.

Do we need to go all the way and merge the two concepts like you suggested on irc? Maybe? It might turn out to be simpler.

Either way, we'll need to figure out how an annotated nullable type behaves in the overload resolution algorithm.

tobie · 2017-03-28T08:52:03Z

index.bs

@@ -6505,6 +6536,9 @@ when passed to a [=platform object=] expecting that type, and how IDL values
 of that type are <dfn id="dfn-convert-idl-to-ecmascript-value" export lt="converted to an ECMAScript value|converted to ECMAScript values">converted to ECMAScript values</dfn>
 when returned from a platform object.

+Note that the sub-sections and algorithms below also apply to the related types created by applying


I don't think we want to do that in every conversion algorithm.

tobie · 2017-03-28T09:06:18Z

index.bs

@@ -8555,6 +8553,66 @@ entails.
 </div>


+<h4 id="LegacyTreatNullAsEmptyString" extended-attribute lt="LegacyTreatNullAsEmptyString" oldids="TreatNullAs">[LegacyTreatNullAsEmptyString]</h4>


Wondering whether the name change belongs in a different PR. I guess this depends on what makes it easier to file and track this in specs and implementations. Thoughts?

This implements the plan discussed in https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0003.html and sets the stage for using extended attributes for SharedArrayBuffer-related purposes. TypeWithAttributes -> TypeWithExtendedAttributes Get rid of [TreatNullAs] on callback interface operations [TreatNullAs=EmptyString] -> [LegacyTreatNullAsEmptyString] Note which extended attributes are applicable to types Allow extended attributes on union member types This wasn't so bad. Grammatically allow extended attributes on dictionary members and arguments (not just on their types) per PR discussion This remains to specify how the types "flow through". Explicitly define the extended attributes associated with a type

domenic · 2017-03-29T10:30:06Z

OK, ready for another pass...

In whatwg/webidl#286 we updated Web IDL so that the extended attributes [Clamp], [EnforceRange], and [TreatNullAs] now apply to types, not attributes or arguments. This updates HTML's usage of those extended attributes to conform to this.

This was not corrected as part of #286.

See whatwg/webidl#286 Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=30107

In whatwg/webidl#286 we updated Web IDL so that the extended attributes [Clamp], [EnforceRange], and [TreatNullAs] now apply to types, not attributes or arguments. This updates HTML's usage of those extended attributes to conform to this.

This updates the grammar for the changes made in whatwg/webidl#286 `[TreatNullAs]` is a legacy attribute and is not yet supported here since nothing uses it on types.

whatwg/webidl#286 makes `[Clamp]`, `[EnforceRange]`, and `[TreatNullAs]` apply to types, this change updates this spec to conform. See also: whatwg/html#2580, whatwg/dom#446

Manishearth · 2019-02-18T09:59:36Z

Updated the File and WebCrypto APIs w3c/FileAPI#116, w3c/webcrypto#218

whatwg/webidl#286 makes `[Clamp]`, `[EnforceRange]`, and `[TreatNullAs]` apply to types, this change updates this spec to conform. See also: whatwg/html#2580, whatwg/dom#446

@nox

Add support for attributes on types in WebIDL WebIDL moved `[Clamp]`, `[RangeEnforced]`, and `[TreatNullAs]` to applying directly to types in whatwg/webidl#286. I implemented parser support for this upstream in [bug 1359269](https://bugzilla.mozilla.org/show_bug.cgi?id=1359269). This pull request downstreams those changes and updates codegen as well as any webidls to conform. Needed to land #22874 r? @nox  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22958)

domenic mentioned this pull request Jan 26, 2017

Add some kind of informative summary table of extended attributes #287

Open

tobie reviewed Jan 30, 2017

View reviewed changes

domenic force-pushed the extended-attrs-on-types branch from 2cda77c to 8a7209a Compare February 3, 2017 23:31

domenic mentioned this pull request Feb 14, 2017

Web GL API safelisting: Web GL 1.0 vs. 2.0 tc39/proposal-ecmascript-sharedmem#168

Open

domenic force-pushed the extended-attrs-on-types branch from 8a7209a to 72bfa37 Compare February 22, 2017 18:33

domenic changed the base branch from gh-pages to master February 22, 2017 18:33

tobie mentioned this pull request Mar 28, 2017

Add examples for extended attribute usage #334

Open

tobie requested changes Mar 28, 2017

View reviewed changes

domenic added 5 commits March 29, 2017 18:44

Use infra for xattrs associated with

f57b662

Make "applicable to types" a dfn

628a6e0

dfn "type being given a new name"

53d585b

Specify that concatenation happens in order

a1f9437

domenic force-pushed the extended-attrs-on-types branch from 72bfa37 to a1f9437 Compare March 29, 2017 09:57

domenic added 3 commits March 29, 2017 19:00

Revert the rename of TreatNullAs

0d68915

Rename to "annotated type" and define that and its inner type

102a664

Deal with overload resolution and unions

d853616

domenic added a commit that referenced this pull request May 10, 2017

Fix [TreatNullAs] usage in example

c0485ed

This was not corrected as part of #286.

domenic mentioned this pull request May 10, 2017

Fix [TreatNullAs] usage in example #360

Merged

tobie pushed a commit that referenced this pull request May 10, 2017

Fix [TreatNullAs] usage in example (#360)

4dbd7d2

This was not corrected as part of #286.

arobins mentioned this pull request May 31, 2017

HTMLFileContent and component for extracting IDL fragments GoogleChromeLabs/webidl-diff#46

Merged

zcorpan added a commit to w3c/csswg-drafts that referenced this pull request Jun 14, 2017

[cssom] Update IDL for [TreatNullAs] extended attribute

ba6d10b

See whatwg/webidl#286 Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=30107

zcorpan mentioned this pull request Jun 14, 2017

[cssom] Update IDL for [TreatNullAs] extended attribute w3c/csswg-drafts#1528

Merged

zcorpan added a commit to w3c/csswg-drafts that referenced this pull request Jun 15, 2017

[cssom] Update IDL for [TreatNullAs] extended attribute

55d5406

See whatwg/webidl#286 Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=30107

Loirooriol pushed a commit to Loirooriol/csswg-drafts that referenced this pull request Jun 18, 2017

[cssom] Update IDL for [TreatNullAs] extended attribute

5730257

See whatwg/webidl#286 Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=30107

tobie mentioned this pull request Sep 19, 2017

Does TreatNullAs apply to DOMString inside a union type? #441

Closed

Manishearth mentioned this pull request Feb 13, 2019

Add support for MediaTrackConstraints servo/servo#22874

Merged

Manishearth mentioned this pull request Feb 18, 2019

Fix Web IDL extended attribute usage: [Clamp] now applies to types w3c/FileAPI#116

Merged

Manishearth mentioned this pull request Feb 18, 2019

Fix Web IDL extended attribute usage: [EnforceRange] now applies to types w3c/webcrypto#218

Merged

bzbarsky mentioned this pull request Feb 23, 2019

It looks like we started allowing [TreatNullAs] on dictionary members #650

Open

domenic mentioned this pull request Feb 25, 2019

Interaction of extended attributes and typedefs is weird #649

Open

Manishearth mentioned this pull request Mar 2, 2019

Add support for attributes on types in WebIDL servo/servo#22958

Merged

lqd mentioned this pull request May 15, 2019

Support attributed types in unions rustwasm/weedle#20

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow extended attributes to apply to types #286

Allow extended attributes to apply to types #286

domenic commented Jan 25, 2017 •

edited by pr-preview bot

Loading

bzbarsky commented Jan 26, 2017

domenic commented Jan 26, 2017

bzbarsky commented Jan 26, 2017

domenic commented Jan 26, 2017

bzbarsky commented Jan 26, 2017

tobie Jan 30, 2017

tobie Jan 30, 2017

domenic Jan 30, 2017

tobie Jan 30, 2017

tobie Jan 30, 2017

domenic Feb 3, 2017

bzbarsky Feb 3, 2017

tobie Jan 30, 2017

domenic Feb 3, 2017

domenic commented Feb 3, 2017

annevk commented Feb 7, 2017

bzbarsky commented Feb 7, 2017

domenic commented Feb 13, 2017

dontcallmedom commented Mar 21, 2017

tobie left a comment

tobie Mar 27, 2017

tobie Mar 27, 2017

tobie Mar 27, 2017

tobie Mar 27, 2017

domenic Mar 29, 2017

domenic Mar 29, 2017

tobie Mar 29, 2017

tobie Mar 27, 2017

tobie Mar 27, 2017

tobie Mar 28, 2017

domenic Mar 29, 2017

tobie Mar 28, 2017

domenic commented Mar 29, 2017

Manishearth commented Feb 18, 2019

		@@ -8555,6 +8553,66 @@ entails.
		</div>


		<h4 id="LegacyTreatNullAsEmptyString" extended-attribute lt="LegacyTreatNullAsEmptyString" oldids="TreatNullAs">[LegacyTreatNullAsEmptyString]</h4>

Allow extended attributes to apply to types #286

Allow extended attributes to apply to types #286

Conversation

domenic commented Jan 25, 2017 • edited by pr-preview bot Loading

bzbarsky commented Jan 26, 2017

domenic commented Jan 26, 2017

bzbarsky commented Jan 26, 2017

domenic commented Jan 26, 2017

bzbarsky commented Jan 26, 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

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

domenic commented Feb 3, 2017

annevk commented Feb 7, 2017

bzbarsky commented Feb 7, 2017

domenic commented Feb 13, 2017

dontcallmedom commented Mar 21, 2017

tobie 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

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

domenic commented Mar 29, 2017

Manishearth commented Feb 18, 2019

domenic commented Jan 25, 2017 •

edited by pr-preview bot

Loading