Type refinements #162

Avaq · 2017-06-26T14:16:44Z

Refinement support:

Helpers:

Strict
NonEmpty

Fixes #128
Fixes #141
Contributes to sanctuary-js/sanctuary#384

davidchambers · 2017-06-26T14:37:04Z

index.js

-  function NullaryTypeWithUrl(name, test) {
-    return NullaryType(name, functionUrl(name), test);
+  //  NullaryTypeWithUrl :: (Type, String, Any -> Boolean) -> Type
+  function NullaryTypeWithUrl(parent, name, test) {


I'd prefer (name, parent, test) for two reasons:

it makes the two constituents of the type's test method, parent and test, adjacent; and

having name first makes the call sites read better, in my view.

davidchambers · 2017-06-26T14:42:38Z

index.js

+    types: {},
+    toString: K('Any')
+  };
+  Any.parent = Any;


We could still leverage NullaryTypeWithUrl here, I believe:

var Any = NullaryTypeWithUrl(null, 'sanctuary-def/Any', null); Any.parent = Any; Any.test = K(true); Any.validate = Right;

davidchambers · 2017-06-26T14:45:04Z

index.js

+  //# None :: Type
+  //.
+  //. Type with no members.
+  var None = NullaryTypeWithUrl(Any, 'sanctuary-def/None', K(false));


Shall we name this Void to match Haskell? None could be confusing due to its use as a data constructor (of type Option a) in Scala.

I'm not sure if we need this. I added it to use in INCONSISTENT only, but we could as well use Any in there since the _test is K(false).

Ah, I see! I like the idea of using Any. :)

davidchambers · 2017-06-26T14:45:51Z

index.js

    'sanctuary-def/FiniteNumber',
-    function(x) { return ValidNumber._test(x) && isFinite(x); }
+    isFinite


This change is particularly satisfying. :)

davidchambers · 2017-06-26T14:47:29Z

index.js

@@ -532,28 +571,35 @@
      $types[k] = {extractor: K([]), type: t};
    });

-    return new _Type(FUNCTION, '', '', format, test, $keys, $types);
+    return new _Type(FUNCTION, '', '', format,
+                     AnyFunction, K(true), $keys, $types);


If it's not possible to have all the arguments on one line, please start each one at the same column. ;)

But my line count! :(

This formatting also satisfies my rule:

return new _Type( FUNCTION, '', '', format, AnyFunction, K(true), $keys, $types );

You're welcome to use this. :)

davidchambers · 2017-06-26T14:50:25Z

index.js

+    Number_,
+    'sanctuary-def/ValidNumber',
+    function(x) { return !isNaN(x); }
+  );


Since it's no longer possible to list the types in alphabetical order, let's decide upon some other sorting rule.

davidchambers · 2017-06-26T14:51:00Z

index.js

+  var TypeClass = NullaryTypeWithUrl(
+    Any,
+    'TypeClass',
+    typeEq('sanctuary-type-classes/TypeClass'));


Please wrap the closing );.

davidchambers · 2017-06-26T14:53:43Z

index.js

-  //. Using types as predicates is useful in other contexts too. One could,
-  //. for example, define a [record type][] for each endpoint of a REST API
-  //. and validate the bodies of incoming POST requests against these types.
+  //. Using types as predicates is can be useful. One could, for example,


s/ is//

I think powerful better captures my thought. How about the following?

Using types as predicates is powerful.

davidchambers · 2017-06-26T14:54:18Z

index.js

@@ -1290,12 +1321,14 @@
  //.
  //. sanctuary-def provides several functions for defining types.

-  //# NullaryType :: String -> String -> (Any -> Boolean) -> Type
+  //# NullaryType :: Type -> String -> String -> (Any -> Boolean) -> Type


Thank you for remembering to update the documentation! 👏

Not the second time ;)

Avaq · 2017-06-26T14:59:30Z

let's decide upon some other sorting rule

We could sort types by their level in the type tree, alphabetically. So:

List of types that have Any as a parent, in alphabetical order.
List of types that refine any of the types from 1, in alphabetical order.
List of types that refine any of the types from 2, in alphabetical order.
...

Avaq · 2017-06-26T15:02:06Z

I prefer grouping "related" types, so perhaps grouping by depth in the type tree is better, we would sort types alphabetically, but ensure that a types children are always listed directly below their parents, that way Number types will be grouped together. It's a slightly more complex rule, but I think will lead to a more readable documentation, eg:

Any
  Boolean
  Function
    Predicate
  Number
    FiniteNumber
    Infinity
...

davidchambers · 2017-06-26T15:04:10Z

I like the idea of grouping related types in the readme. It would be nice to have:

Number
ValidNumber
NonZeroValidNumber
FiniteNumber
NonZeroFiniteNumber
…

I'm not sure how to codify the internal rule which makes this ordering feel natural to me.

The sheme has been discussed in #162

Avaq · 2017-06-27T09:01:30Z

What do you think of this new order:

Any
  AnyFunction
  Arguments
  Array
  Boolean
  Date
    ValidDate
  Error
  Inconsistent
  Null
  Number
    ValidNumber
      FiniteNumber
        NegativeFiniteNumber
        NonZeroFiniteNumber
        PositiveFiniteNumber
      Integer
        NegativeInteger
        NonZeroInteger
        PositiveInteger
      NegativeNumber
      NonZeroValidNumber
      PositiveNumber
  Object
  RegexFlags
  RegExp
    GlobalRegExp
    NonGlobalRegExp
  String
  Symbol
  Type
  TypeClass
  Undefined
  Unknown
Function
NonEmpty
Nullable
Pair
StrMap

Avaq · 2017-06-29T11:37:12Z

index.js

+  //. tuples. `['foo', 42]` is a member of `Pair String Number`.
+  var Pair = BinaryTypeWithUrl(
+    'sanctuary-def/Pair',
+    Array_(Any),


Not sure whether to use Array Any or Array Unknown here. I think using Any is fine.

Array Any is clearer to me. 👍

Avaq · 2017-06-29T12:05:04Z

I think we should hold off implementing this for record types, I think it would be better to leave record types refine Object. I think "extending" record types the way you suggested in #128 should be done in a different way. It could have the same API, but not be implemented on top of refinements.

With that out of the way, I think this PR is now "ready".

davidchambers · 2017-06-29T14:14:43Z

I think "extending" record types the way you suggested in #128 should be done in a different way.

Why do you think so?

davidchambers · 2017-06-29T14:35:31Z

index.js

-  //  Inconsistent :: Type
-  var Inconsistent =
-  new _Type(INCONSISTENT, '', '', always2('???'), K(false), [], {});
-


What's the thinking behind moving this definition? Did it feel inconsistent having it apart from the others?

It depends on Any being defined.

davidchambers · 2017-07-04T17:50:05Z

test/index.js

-           'create :: { checkTypes :: Boolean, env :: Array Any } -> Function\n' +
-           '          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n' +
+           'create :: { checkTypes :: Boolean, env :: Array Type } -> Function\n' +
+           '          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n' +


This is a nice improvement. :)

davidchambers · 2017-07-04T17:54:52Z

test/index.js


    //  Point :: Type
-    var Point = $.RecordType({x: $.Number, y: $.Number});
+    var Point = $.RecordType($.Any, {x: $.Number, y: $.Number});


Every record type defined in the test suite currently extends $.Any. It would be good to include your Point3D example here as well.

Avaq

My thoughts regarding the unaddressed feedback.

Avaq · 2017-07-05T09:37:40Z

index.js

+  //.
+  //. The `p2` value is a member of `Point3D` as well as `Point`, making it a
+  //. valid argument to `dist`. By default record types allow additional
+  //. properties (which is what allowed `p2` to be a member of `Point`). If one


I want readers to be aware that permitting additional properties is a prerequisite to allowing record extension as described in the prior section. Maybe this more concrete clause is more to your liking?

I ❤️ the concrete clause. In fact, I would lead with it:

By default, record types permit the presence of additional fields. As a result, p2 is a member of Point as well as Point3D.

One could define Strict :: Type -> Type and use it to define record types which do not permit additional fields:

Avaq · 2017-07-05T09:47:42Z

index.js

+  //.   '',
+  //.   t,
+  //.   x => Object.keys(x).length === t.keys.length
+  //. );


How do you propose this be made into a UnaryType? I see one hacky way:

// Strict :: Type -> Type const Strict = t => $.UnaryType( 'Strict' + t.name, t.url, t, x => Object.keys(x).length === t.keys.length, x => [x] )(t); //<- That's unfortunate

The "proper" way leads to a dead end:

// Strict :: Type -> Type const Strict = $.UnaryType( 'my-package/Strict', //<- That's unfortunate 'http://example.com/#strict', //<- That's unfortunate $.Any, //<- That's unfortunate x => Object.keys(x).length === `???`, //<- That's impossible x => [x] );

I currently think strict makes more sense as a "common refinement" (which justifies its inclusion in the library), and I probably would have also made nonEmpty in that way.

The advantage of having NonEmpty be a UnaryType is that "abc" is automatically a member of it, correct? So we don't have to explicitly create NonEmptyString and add it to the environment.

Maybe the test could be something along the lines of:

x => { const len = Object.keys(x).length; return determineActualTypesStrict(x).every(type => type.keys.length === len); }

The first "hacky" approach will not have the same benefits as NonEmpty. We have to find an approach where Strict(Unknown) has members.

Altough, I don't have to add my Strict type to the environment anyway, I can create it at run time without drawback, I think:

strict(User).validate(userInput)

const createUser = def('createUser', {}, [strict($User), $Future($Error, $UserId)], Future.encase2(mongo.insertOne, 'users'))

How about this approach?

// Strict :: Type -> Type const Strict = $.UnaryType( 'my-package/Strict', 'http://example.com/my-package#Strict', $.Any, function(x) { const t = this.types.$1.type; return t.type === 'RECORD' && $.test([], t, x) && Object.keys(x).length === t.keys.length; }, strict => [strict] );

Woah. Using this inside a test function is new, correct? There are some places where we're passing t._test down into a new Type, I'm not sure if the consequences of relying on this will include breakage of code relying on the "purity" (this-lessness?) of _test. If it doesn't, then the fact that we can use this to refer to the type of a value being tested is actually quite neat.

I do still doubt whether using UnaryType for this purpose is useful though. When implementing this as a type refinement, we get the elegance of strict(User) being a refinement of User. When implementing this as a UnaryType, I'm not sure if we gain anything; I believe that adding Strict(Unknown) to the environment will not cause {name: "Bob"} :: User, Strict User, because Unknown.type !== 'RECORD'. This differs from NonEmpty, as its check does not rely on the type of types.$1.

Referencing this does feel naughty, I agree. Let's stick with the definition you have, but capitalize the identifier in accordance with our convention of capitalizing the names of types and type constructors.

I have a much stronger argument:

> var User = $.RecordType($.Any, {name: $.NonEmpty($.String)}) > User.validate({name: ''}) Left({ value: '', propPath: [ 'name' ] }) > strict(User).validate({name: ''}); Left({ value: '', propPath: [ 'name' ] }) > Strict(User) Left({ value: { name: '' }, propPath: [] })

Or in words: Your UnaryType relies on $.test, and the purpose of parent types was to fix validate for those types that relied on test-ing another type. Actually, the purpose of this PR was to fix #128, and in particular, to address my comment there: #128 (comment)

davidchambers · 2017-07-05T14:43:47Z

index.js

@@ -2538,6 +2668,7 @@
    PositiveNumber: PositiveNumber,
    RegExp: RegExp_,
    RegexFlags: RegexFlags,
+    Strict: Strict,
    StrMap: fromUncheckedUnaryType(StrMap),
    String: String_,


I've been using Array#sort to determine order:

> ['Strict', 'StrMap', 'String'].sort() ['StrMap', 'Strict', 'String']

Please also move the definition of Strict.

I tried, but found that my editors built-in sorting mechanism changed the position of String. I'll switch to use Array#sort. :)

davidchambers · 2017-07-05T14:45:10Z

test/index.js

@@ -1345,6 +1357,22 @@ describe('def', function() {
           'Since there is no type of which all the above values are members, the type-variable constraint has been violated.\n');
  });

+  it('support "strict" record types', function() {


s/support/supports/

davidchambers · 2017-07-05T14:48:01Z

test/index.js

@@ -1345,6 +1357,22 @@ describe('def', function() {
           'Since there is no type of which all the above values are members, the type-variable constraint has been violated.\n');
  });

+  it('supports "strict" record types', function() {
+


For consistency, please remove this empty line and the one before the closing });.

davidchambers · 2017-07-05T14:49:44Z

test/index.js

+    var p = {x: 1, y: 2, z: 3};
+
+    eq($.test([], Point, p), true);
+    eq($.test([], StrictPoint, p), false);


I'd like to see four assertions here:

eq($.test([], Point, {x: 1, y: 2}), true); eq($.test([], Point, {x: 1, y: 2, z: 3}), true); eq($.test([], StrictPoint, {x: 1, y: 2}), true); eq($.test([], StrictPoint, {x: 1, y: 2, z: 3}), false);

davidchambers · 2017-07-05T14:55:13Z

index.js

+  //. Constructor for strict record types. For example:
+  //. `$.Strict($.Record($.Any, {name: $.String}))`, is the type comprising
+  //. every object with a `name :: String` property, without any other
+  //. properties.


Do you like this variant?

Constructor for strict record types.

$.Strict($.Record($.Any, {name: $.String})), for example, is the type comprising every object with exactly one field, name, of type String.

davidchambers · 2017-07-05T14:56:15Z

index.js

+  //. properties.
+  function Strict(t) {
+    function test(x) {
+      return Object.keys(x).length === t.keys.length;


Object.keys ignores inherited properties. Is this desirable?

I thought about that, and I don't think we can make a sensible Strict constructor otherwise.

Is using for...in impractical for some reason?

function O() {} var o = new O(); o.direct = true; O.prototype.indirect = true; Object.keys(o); // => ['direct'] (function() { var keys = []; for (var k in o) keys.push(k); return keys; }()); // => ['direct', 'indirect']

We use keys.every(k => k in o) as a test for record types. Will this lead to problems?

> 'then' in new Promise(() => {}) true > (function(o) { var keys = []; for (var k in o) keys.push(k); return keys; }(new Promise(() => {}))) []

This would mean that:

const Thenable = $.RecordType({then: $.AnyFunction}); const StrictThenable = $.Strict(Thenable); Thenable.validate(new Promise(noop)) // => Right StrictThenable.validate(new Promise(noop)) // => Left

I'm not sure if that's desired. Using enumerable keys over "own" keys is an improvement though, so I think I should implement your suggestion nonetheless.

davidchambers · 2017-07-05T14:57:53Z

index.js

+  //. The `p2` value is a member of `Point3D` as well as `Point`, making it a
+  //. valid argument to `dist`. By default, record types permit the presence
+  //. of additional fields. As a result, p2 is a member of Point as well as
+  //. Point3D.


p2, Point, and Point3D should be wrapped in backticks.

Darned copy-paste ;)

Avaq · 2017-07-10T11:09:11Z

index.js

-  );
+    }
+    function extract(monoid) { return [monoid]; }
+    var t = UnaryTypeWithUrl('sanctuary-def/NonEmpty', parent, test, extract);


Using parent as an actual parent type, like I'm doing here, seems like a good idea. Now NonEmpty String will (correctly) be a "refinement" of String.

However, as indicated by the failing unit test, it causes a value of [1] for $NonEmpty($Array($String)), for example, to generate this error message:

NonEmpty (Array String) ^^^^^^^^^^^^^^ 1 1) 1 :: Number The value at position 1 is not a member of ‘Array String’.

Where we would want:

NonEmpty (Array String) ^^^^^^ 1 1) 1 :: Number The value at position 1 is not a member of ‘String’.

I'm not sure why this happens. Replacing parent with Any solves the issue.

The problem is that

$.NonEmpty($.Array($.String)).validate([1, 2, 3]).value.propPath

evaluates to ['$1'] rather than ['$1', '$1'] because we first validate the parent type and, if validation fails, make no adjustment to propPath to account for the extra level of nesting introduced by $.NonEmpty.

That seems like an issue that will bite us in some other way, one day. I don't yet see a solution.

In it's general form, I think the issue could be described as: When the validation of a refined type fails on its parent type, the reported propPath may not correctly correspond to the location of the value as it exists in the refined type

We even encounter this issue if we refine a record type to be "strict", as has always been the suggestion:

const User = $.RecordType({ name: $.String, password: $.String }); const StrictUser = $.NullaryType('', '', User, x => Object.keys(x).length === User.keys.length); const validation = StrictUser.validate({ name: 'bob', password: 1 }); // propPath = [ 'password' ] StrictUser.types[validation.value.propPath[0]] // => undefined // the above expression should never return undefined, as the propPath must // correspond to the types mapping. It does because our propPath is actually // corresponding to another types mapping!

The same problem, biting us somewhere else:

const Point = $.RecordType($.Any, { x: $.Number, y: $.Number }); const Point3D = $.RecordType(Point, { z: $.Number }); const validation = Point3D.validate({ x: 1, y: 'two', z: 3 }); // propPath = [ 'y' ] Point3D.types[validation.value.propPath[0]] // => undefined

This is particularly problematic, because the property names described by propPath might exist in both the parent type and the refinement, and validate does not tell us which it was.

davidchambers · 2017-12-07T18:28:31Z

Do you remember where we left this, Aldwin? In light of sanctuary-js/sanctuary#464 I'm very excited by the possibility of merging this pull request. :)

Avaq · 2017-12-08T16:48:12Z

Hi David. I don't think this will be ready to merge anytime soon. We left it at the point where we realised that this brings more problems than it solves. Here's some exempts from our Gitter conversation at the time:

I'm having trouble again with the Strict/NonEmpty case. It seemed to have a clear solution last time:

We created type refinements with the purpose of defining strict record types in terms of them, therefore it goes without saying that Strict should be a helper function that takes away the boilerplate of doing so.

We then moved on to conclude that NonEmpty is very similar to Strict, and should probably follow the same logic. However, after defining NonEmpty this way, I've run into an issue that could not be encountered with Strict, to do with t.name. Look at this definition:
 function NonEmpty(t) {
   return NullaryType('sanctuary-def/NonEmpty',
                      functionUrl('NonEmpty'),
                      t,
                      function test(x) {
                        return Z.Monoid.test(x) &&
                               Z.Setoid.test(x) &&
                               !Z.equals(x, Z.empty(x.constructor));
                      });
 }
This will cause the following:
const head = def('head', {}, [$NonEmpty($Array($a)), $a], xs => xs[0]);
head.toString()
-head :: NonEmpty (Array a) -> a
+head :: NonEmpty -> a
We have not run into this issue with Strict, simply because we don't render the name of record types, and have not tested against it. However, I suspect that after making a record type "Strict", it will no longer be rendered very informatively in error messages. All of this leads me to conclude that:

Using a UnaryType for NonEmpty makes sense, because we keep all type information for the inner type.

Using a NullaryType refinement for Strict is not good enough. Especially when we ever add names to record types.

I'm going to explore using a UnaryType for both NonEmpty and Strict, and see if I can still get them to be a refinement as well as a "container" of the input type.

I've modified Type#validate to include a expType :: Type field in the returned record, for example in $.Integer.validate('hello'), this would have a value of $.Number, because that's the actual type for which our validation failed. The propPath then always applies to the expType, and we can even deduce that:
1) "hello" :: String

The value at position 1 is not a member of Integer, because it is not a Number.
...for example. But there are a lot of changes to be made to the code which builds on Type#validate before we are at that point.

I hope you're able to find a way to make $.Strict a unary type.

Type refinements worked so well on the surface, but upon taking a closer look I find that many things are broken under the current approach. Take this for example:
const Point = $.RecordType($.Any, {x: $.Number, y: $.Number});
const Point3D = $.RecordType(Point, {z: $.Number});

const StrictPoint3D = $.Strict(Point3D);
We haven't considered that StrictPoint3D will have 0 members. The Strict wrapper expects an object with one key, because Point3D.keys.length === 1, and when an object with one key is passed, it's invalidated by the parent, which expects two keys.

It's a similar problem to what we faced before. The meta-data on Point3D does not reflect the "full" meta-data of the combined Point+Point3D.

It seems we need to walk the tree to construct the union ourselves. We'll also need to decide what to do if we encounter a contradiction (e.g. {foo: $.Number} and {foo: $.String}).

That's the last few thoughts we had on the subject, and where things came to an unfortunate halt.

Incidentally, our Gitter conversation is a treasure trove of ideas! Here's some:

I called env: remove refinement types from default environment sanctuary#464 in July!

I've been thinking about the purpose of env, and there is one burning question that arises: Why does sanctuary add types like $.FiniteNumber to the environment? [...] We're adding types to the environment for the sole purpose of showing nicer error messages. For the purpose of type variables, the only types that need to be in the environment are direct descendants of Any. [This] means we can optimize the amount of checks we're doing by ignoring any types where t.parent !== Any.

And when it comes to reporting all known types of a value, why do we limit ourselves the the environment? It starts to become attractive to simply add all types to the environment. The more types Sanctuary knows about, the more precisely an error message will report what the types of a value are at no extra run-time cost[, because of the previous idea].
This change to the toMaybe API to work more like encase. You asked me to open an issue and I forgot, so here it is: Change toMaybe API to work more like encase sanctuary#465

davidchambers · 2019-04-05T19:54:34Z

Shall we close this, Aldwin?

Avaq · 2019-04-06T09:35:52Z

Let's do it. The effort is not lost, because it has gotten me familiar with the sanctuary-def code. :)

Avaq force-pushed the avaq/type-refinements branch 2 times, most recently from 02f1592 to 1a34c34 Compare June 26, 2017 14:25

davidchambers reviewed Jun 26, 2017

View reviewed changes

Avaq force-pushed the avaq/type-refinements branch from 1a34c34 to f0e8340 Compare June 26, 2017 14:53

davidchambers reviewed Jun 26, 2017

View reviewed changes

Avaq force-pushed the avaq/type-refinements branch 2 times, most recently from e23e6e1 to d2e90df Compare June 26, 2017 14:55

Add parent type argument to NullaryType

50e9164

Avaq force-pushed the avaq/type-refinements branch from d2e90df to 50e9164 Compare June 26, 2017 14:56

Reorder types based on new ordering scheme

e983ea2

The sheme has been discussed in #162

Add refinement support to UnaryType constructor

c5f1130

Avaq force-pushed the avaq/type-refinements branch from 769fd41 to c5f1130 Compare June 27, 2017 09:45

Add refinement support to BinaryType constructor

90b7aa3

Avaq commented Jun 29, 2017

View reviewed changes

Remove no longer necessary null-check

fa55a10

Avaq changed the title ~~WIP: Type refinements~~ Type refinements Jun 29, 2017

davidchambers reviewed Jun 29, 2017

View reviewed changes

davidchambers reviewed Jul 4, 2017

View reviewed changes

Aleviate minor complaints about documentation

1ae62f4

Avaq commented Jul 5, 2017

View reviewed changes

In-line Options record definition with create definition

4b90668

davidchambers reviewed Jul 5, 2017

View reviewed changes

Add Strict type constructor

9708db1

Avaq force-pushed the avaq/type-refinements branch from 4ec5ba4 to 9708db1 Compare July 5, 2017 14:46

davidchambers reviewed Jul 5, 2017

View reviewed changes

Rephrase documentation on additional record fields

cc92cd2

Avaq force-pushed the avaq/type-refinements branch from 94ed207 to cc92cd2 Compare July 5, 2017 14:51

davidchambers reviewed Jul 5, 2017

View reviewed changes

Avaq added 2 commits July 5, 2017 17:00

Incorporate PR feedback

2fce7d9

Take all enumerable properties into account for strict record types

f1da1c4

Avaq mentioned this pull request Jul 10, 2017

decide what qualifies as a "field" #164

Closed

Avaq added 2 commits July 10, 2017 10:51

Let record and strict types acknowledge all enumerable properties

e08290c

WIP: Level Strict and NonEmpty in terms of approach

7b57bc0

Avaq commented Jul 10, 2017

View reviewed changes

davidchambers mentioned this pull request Dec 7, 2017

env: remove refinement types from default environment sanctuary-js/sanctuary#464

Closed

Avaq mentioned this pull request Mar 27, 2019

support explicit subtyping #236

Merged

Avaq closed this Apr 6, 2019

davidchambers deleted the avaq/type-refinements branch April 6, 2019 17:54

Type refinements #162

Type refinements #162

Conversation

Avaq commented Jun 26, 2017 • edited Loading

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

Choose a reason for hiding this comment

Avaq commented Jun 26, 2017 • edited Loading

Avaq commented Jun 26, 2017 • edited Loading

davidchambers commented Jun 26, 2017

Avaq commented Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Avaq commented Jun 29, 2017

davidchambers commented Jun 29, 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

Avaq 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

Avaq Jul 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Avaq Jul 5, 2017 • edited Loading

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

Avaq Jul 5, 2017 • edited Loading

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

Avaq Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

Avaq Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidchambers commented Dec 7, 2017

Avaq commented Dec 8, 2017 • edited Loading

davidchambers commented Apr 5, 2019

Avaq commented Apr 6, 2019

Avaq commented Jun 26, 2017 •

edited

Loading

Avaq commented Jun 26, 2017 •

edited

Loading

Avaq commented Jun 26, 2017 •

edited

Loading

Avaq commented Jun 27, 2017 •

edited

Loading

Avaq Jul 5, 2017 •

edited

Loading

Avaq Jul 5, 2017 •

edited

Loading

Avaq Jul 5, 2017 •

edited

Loading

Avaq Jul 10, 2017 •

edited

Loading

Avaq Jul 10, 2017 •

edited

Loading

Avaq commented Dec 8, 2017 •

edited

Loading