Split the registry into registry, web, and storage services #135

chrisdickinson · 2019-06-04T06:23:39Z

Status: it's ready!

ceejbot · 2019-06-04T15:15:31Z

package.json

@@ -31,8 +31,8 @@
    "lint-cli": "cd cli; npm run lint",
    "lint-fix": "prettier --write '**/*.js'",
    "lint-registry": "cd services/registry; npm run lint",
-    "postinstall": "for d in cli services/registry services/workers services/common/boltzmann; do cd $d; npm i; cd -; done",
+    "postinstall": "for d in cli services/registry services/workers services/web services/common/boltzmann; do cd $d; npm i; cd -; done",
    "start": "./misc/start.sh",


Mostly I'm just thinking about how much work is ahead re-implementing run-scripts.

ceejbot · 2019-06-04T15:16:06Z

services/common/boltzmann/middleware/requestid.js

  return function mw(next) {
    return async function inner(context) {
      const request = context.request;
-      context.id = request.headers[requestIdHeader] || uuid.v1();
+      context.id = request.headers[requestIdHeader] || `${host}_${uuid.v1()}`;


Aha. Excellent.

ceejbot · 2019-06-04T15:17:26Z

services/web/NOTES

+    deleteToken({for: user, valueHashes: [String]}) -> { count }
+    resolveCLISession({session, value})
+    fetchCLISession({session}) -> { description }
+


This is, I note, the minimal useful web feature set. I think it's smart of you to avoid things like package visualization for now.

ceejbot · 2019-06-04T15:19:44Z

services/web/handlers/auth.js

+  fork.get('/signup', redirectAuthenticated(signup)),
+  fork.post('/signup', redirectAuthenticated(signupAction)),
+  fork.get('/tokens', seasurf(redirectUnauthenticated(tokens))),
+  fork.post('/tokens', seasurf(redirectUnauthenticated(handleTokenAction)))


Yeah, putting the endpoints into registry for now is good. You might consider namespacing them. I think that'll be good even after we pull things into a second service.

Ideally this lands at /settings/:username/tokens, but I'm going to leave that for another PR.

ceejbot · 2019-06-04T15:22:04Z

services/web/handlers/auth.js


-    context.session.set('banner', 'Successfully deleted 1 token.');
+    context.session.set('banner', `Successfully deleted ${count} token${count === 1 ? '' :  's'}.`);


the thin edge of the wedge with ternaries

ceejbot · 2019-06-04T15:23:55Z

services/web/package.json

@@ -0,0 +1,74 @@
+{
+  "name": "entropic-web",
+  "description": "community package manager but for spiders",


ceejbot · 2019-06-04T15:24:37Z

services/web/server.js

+} else {
+  bole.output({ level: 'info', stream: process.stdout });
+}
+


I note that 90% of this is boilerplate. We should make a generic wrapper.

My go at code golfing the wrapper:

Wrapper

function boltzStarter(router, middles, boltzmann) { return boltzmann.make(router, middles.map(middle => require(middle))); }

Usage

const myMiddles = [ 'boltzmann/middleware/logger', 'boltzmann/middleware/flush-request', 'boltzmann/middleware/requestid', 'boltzmann/middleware/redis', 'boltzmann/middleware/storage-api', './middleware/session', ]; boltzStarter(router, myMiddles, boltzmann).listen( process.env.PORT, '0.0.0.0' );

Oh, I commented above but it looks like y'all are already on it.

I made a ticket for that here so it doesn't get lost

chrisdickinson · 2019-06-07T06:01:06Z

services/common/boltzmann/client/NOTES

@@ -0,0 +1,100 @@
+storageApi


Hey look, it's almost a complete list of everything an entropic can do!

chrisdickinson · 2019-06-07T06:03:05Z

services/common/boltzmann/client/index.js

@@ -0,0 +1,207 @@
+'use strict'


Both the web gateway and registry gateway can use this client to talk to storage.

chrisdickinson · 2019-06-07T06:03:49Z

services/common/boltzmann/client/index.js

+  }) {
+    this.url = url
+    this.requestId = requestId
+    this.userAgent = `${hostname}(${userAgent})`


Wrap up the incoming requestId and userAgent (so we know which host is sending which user-agent where, in logs.)

Should I add x-forwarded-for here as well, so remote address logging works correctly in storage-api?

chrisdickinson · 2019-06-07T06:04:36Z

services/common/boltzmann/client/index.js

+      body: rawBody ? rawBody : body ? JSON.stringify(body) : null
+    })
+
+    if (!response.ok) {


The fetch api is so nice.

chrisdickinson · 2019-06-07T06:04:57Z

services/common/boltzmann/request-handler.js

+      return this._parsedUrl
+    }
+    this._parsedUrl = parse(this.request.url, true)
+    return this._parsedUrl


Be lazy about URL parsing.

chrisdickinson · 2019-06-07T06:06:21Z

services/registry/handlers/maintainers.js

+      'maintainer.invite.package_dne': `Unknown package: "${invitee}".`,
+      'maintainer.invite.already_accepted': `Namespace "${invitee}" is already a member.`,
+      'maintainer.invite.already_declined': `Namespace "${invitee}" has declined this invite.`
+    }[err.code]


The error mapping pattern emerges. It's almost worth making a decorator for this and throwing the error, I think. This is where we should hang i18n mapping as well, potentially.

chrisdickinson · 2019-06-07T06:08:59Z

services/registry/middleware/bearer-auth.js

+      const hash = crypto
+        .createHash('sha256')
+        .update(bearer + process.env.SESSION_SECRET)
+        .digest('base64')


The registry now caches auth in redis for a span of time, so we hash it.

chrisdickinson · 2019-06-07T06:09:32Z

services/registry/package.json

    "culture-ships": "~1.0.0",
    "dotenv": "~8.0.0",
    "escape-html": "^1.0.3",
-    "find-my-way": "~2.0.1",
    "graceful-fs": "^4.1.15",
    "is-email-maybe": "^1.0.1",
    "markdown": "^0.5.0",


TODO: review all service deps and shrink to the minimum set for each service.

TODO another day, perhaps

writing up issue

chrisdickinson · 2019-06-07T06:13:19Z

This turned out to be much bigger than expected! (It's good to get this out of the way now, though, because it's just going to move more cheese as time goes on.)

Things left to do:

Finish wiring up the client
Fill out the remaining "TODO" api endpoints from the client (things we were previously glomming together with the registry/web endpoints before, like token creation)
~~Do a error enumeration pass~~ for now, pass errors through and leave TODO statements in an effort to not bite off more than I can chew with this PR.
Do a logging pass through:
- the registry
- the website
- the storage layer

services/common/boltzmann/client/index.js

olingern · 2019-06-08T01:21:00Z

services/common/boltzmann/client/index.js

+
+const hostname = os.hostname()
+
+const e = encodeURIComponent


What do you think of wrapping this in a function? If any future refactors need to happen it could make it easier.

const encode = (str) => encodeURIComponent(str)

olingern · 2019-06-08T01:22:03Z

services/common/boltzmann/client/index.js

+
+module.exports = class Client {
+  constructor ({
+    url = process.env.STORAGE_API_URL,


Should we assert, throw, or warn here if that env var isn't defined?

olingern · 2019-06-08T01:49:26Z

services/common/boltzmann/client/index.js

@@ -185,9 +300,10 @@ module.exports = class Client {
        message = parsed.message || body
        code = parsed.code || code
      } catch {
+        ;


I'm sure you're probably working on this now, but commenting just in case. Empty line / no caught error

olingern · 2019-06-08T01:55:59Z

services/storage/handlers/namespaces.js

+        'namespace_members.active': true,
+        'namespace_members.user_id': context.user.id
+      })
+      .catch(Namespace.objects.NotFound, () => null);


Could we add a log here? #202 took me a bit to track down 😅

This should appear a lot noisier in the logs now after adding dev-mode traces to error responses.

olingern · 2019-06-08T01:59:21Z

Awesome work! I thought I would try to help PR review and leave some comments. I'll try to test this branch out at some point to see if I can turn up any 🪲

zacanger · 2019-06-08T02:02:29Z

services/common/boltzmann/request-handler.js

@@ -1,5 +1,7 @@
 'use strict';

+const { parse } = require('url');


url.parse is deprecated, should this use new URL instead?

Ah yeah. new URL has the (uncomfortable in this case) prereq that you have to give it two args to prevent it from throwing if the first arg is an "incomplete" url – we probably should move over to it, but as a follow up PR.

Noting to self that I should write up that task as an issue and tag it as a good starter issue.

zacanger · 2019-06-08T02:05:16Z

services/registry/handlers/maintainers.js

+    page: Number(context.url.query.page) || 0,
+    status: context.url.query.status
+  }).then(
+    xs => [null, xs],


I like this. Haskell influence?

Haha, thanks! I got it from looking at Golang a while back, tbh.

zacanger · 2019-06-08T02:06:34Z

services/registry/handlers/users.js

+    namespace,
+    host
+  }).then(
+    xs => [null, xs],


Would it be worth pulling this into an either util?

That would be great once this lands!

zacanger · 2019-06-08T02:13:36Z

services/storage/handlers/packages.js

+'use strict';
+
+const { Response } = require('node-fetch');
+const { markdown } = require('markdown');


This particular markdown lib is very abandoned. It might make sense to use something like markdown-it, which is both maintained and super pluggable.

I was waiting to use marky-markdown, which is a Known Good package for rendering markdown; however it brings along oniguruma so I wanted to wait until we had separated package publishes out into a worker process.

Added a ticket so this doesn't get lost.

services/storage/handlers/packages.js

chrisdickinson · 2019-06-08T07:06:02Z

OK, I'm calling it – this PR is ready to merge, I think. Brace yourself: I anticipate that it's going to break stuff and things will be a bit bumpy for a little while.

Here's what I've done:

www.js has been split out of registry into services/web.
Most of the registry code has moved back up into a new (private) service called storage.
- The intent is that this will not be visible to the outside world.
web and registry are responsible for trading external authentication (session cookies and authorization headers, respectively) for internal authentication (the surprisingly versatile bearer: <username> header.)
registry caches authentication results in redis.
membership (of users to namespaces) and maintainership (of packages by namespaces) URLs have been restructed but the CLI has not caught up. See URL CHANGES below.
docker-compose.yml contains all the new services.
Added response.error.coded which accepts a string (a la namespace.members.bearer_unauthed), designed to create a disjoint between storage layer error cases and registry layer error message display. (This means we can start pushing i18n concerns into the registry layer!)

I've tested:

logging in both via services running locally and through docker
publishing a new package
testing whoami
manually curling a few list endpoints – packages, maintainerships, memberships.

I have not:

updated the documentation (for another PR!)
added registry tests (but wow we need them! all of the existing tests – save invitations, which were deleted due to massive upheaval – moved from registry to storage)
audited the dependencies of each service. It's possible (likely!) that services are depending on things they aren't using.

URL CHANGES

GET     /v1/users/user/:user/memberships[?pending=true]                list membership
POST    /v1/users/user/:user/memberships/invitations/:namespace@:host  accept invitation
DELETE  /v1/users/user/:user/memberships/invitations/:namespace@:host  decline invitatation

GET     /v1/namespaces/namespace/:namespace@:host/maintainerships[?pending=true]            list maintainerships
POST    /v1/namespaces/namespace/:namespace@:host/maintainerships/:namespace@:host/:package accept invite
DELETE  /v1/namespaces/namespace/:namespace@:host/maintainerships/:namespace@:host/:package decline invite

GET     /v1/namespaces/namespace/:namespace@:host/members[?pending=true] list members
POST    /v1/namespaces/namespace/:namespace@:host/members/:user          invite user
DELETE  /v1/namespaces/namespace/:namespace@:host/members/:user          remove member / rescind invite

GET     /v1/packages/package/:namespace@:host/maintainers[?pending=true] list maintainers
POST    /v1/packages/package/:namespace@:host/maintainers/:name@:host    invite maintainer
DELETE  /v1/packages/package/:namespace@:host/maintainers/:name@:host    remove maintainer / rescind invite

This eliminates all urls with the word invite in them, because we don't so much have invites as "pending" maintainerships and memberships; and the user/namespace is free to leave if they choose to do so. The CLI should be updated to reflect these changes! This can be done in a subsequent PR.

why is it important to move all this cheese now

Moving this cheese:

Gives people a service in which to build all the website functionality in a connected way. (If we end up going with Next.js, we may need yet another service that lives outside of the private network and uses web as an API gateway/backend-for-frontend. That's okay, though – we've already gone from 1 service to 3. 3 to 4 will be much easier.)
Lets us separate expensive processes (like publishing and syncing!) into a worker process.

chrisdickinson · 2019-06-08T07:06:39Z

package.json

    "start": "docker-compose up",
-    "test": "for d in cli services/registry; do cd $d; npm t; cd -; done"
+    "test": "for d in cli services/{registry,workers,web,common/boltzmann}; do cd $d; npm t; cd -; done"


Use bash in anger.

This package.json is a makefile, to be quite honest.

chrisdickinson · 2019-06-08T07:07:18Z

services/common/boltzmann/client/NOTES

+    getObject({algo, digest}) // promise for a stream
+
+
+response.rawjson(str) // for enemies


Leaving this for now, will clean up afterward. Sectumsempra!

chrisdickinson · 2019-06-08T07:08:27Z

services/common/boltzmann/response.js

+
+  if (isDev()) {
+    err.trace = new Error().stack;
+  }


If we're in dev mode, add a trace property to all outgoing errors. This makes it easier to track down errors when they happen: the registry client will log traces coming from storage.

Thoughtful!

chrisdickinson · 2019-06-08T07:08:58Z

services/common/boltzmann/response.js

  const r = new Response(JSON.stringify(err), { status, headers });
  return r;
 }

+error.coded = coded;
+function coded(code, ...args) {
+  return error(Object.assign(new Error(code), { code }), ...args);


For storage-layer use. Takes computer-friendly dotted, lowercase error codes.

chrisdickinson · 2019-06-08T07:09:22Z

services/common/boltzmann/response.js

 function authneeded(message, status = 401, extraHeaders = {}) {
  const headers = new Headers({
    'www-authenticate': 'bearer',
    'content-type': 'application/json',
    ...extraHeaders
  });
  if (typeof message === 'string') {
-    message = { error: message };
+    message = { message, code: 'authneeded' };


Standardize a bit on message (which is used in response.error), while we're at it.

I am good on standardizing on this. Fallback English message + code as a localization switch point. I note that at some point we'll want to pull error codes into some kind of known constants list, for re-use.

chrisdickinson · 2019-06-08T07:22:33Z

services/storage/models/authentication.js

-    );
-  }
-}
-


Provider data lives in web now.

chrisdickinson · 2019-06-08T07:23:15Z

services/storage/models/authentication.js

@@ -96,6 +59,8 @@ module.exports = class Authentication {
  }
 };

+const User = require('./user');


Had to shuffle requires because of 🎶 circular imports 🎶

chrisdickinson · 2019-06-08T07:23:39Z

services/storage/models/token.js

@@ -34,6 +34,11 @@ module.exports = class Token {
    this.active = active;
  }

+  toJSON() {
+    const { user: _0, id: _1, user_id: _2, ...meta } = this;


Omitting properties by destructure+splat.

chrisdickinson · 2019-06-08T07:24:42Z

services/web/handlers/auth.js

+  fork.get('/signup', redirectAuthenticated(signup)),
+  fork.post('/signup', redirectAuthenticated(signupAction)),
+  fork.get('/tokens', seasurf(redirectUnauthenticated(tokens))),
+  fork.post('/tokens', seasurf(redirectUnauthenticated(handleTokenAction)))


Ideally this lands at /settings/:username/tokens, but I'm going to leave that for another PR.

chrisdickinson · 2019-06-08T07:26:35Z

services/web/handlers/index.js

+    <!doctype html>
+    <html>
+      <body>
+        <h1><marquee>WELCOME TO ENTROPIC</marquee></h1>


It's a package registry on the move!

ceejbot

I had some comments but nothing that needs action in this PR.

ceejbot · 2019-06-08T13:18:23Z

.circleci/config.yml

    docker:
      - image: circleci/node:latest
      - image: circleci/postgres:latest

-    working_directory: ~/repo/services/registry
+    working_directory: ~/repo/services/storage


ceejbot · 2019-06-08T13:19:06Z

docker-compose.yml

+    command: npm start
+    networks:
+      - entropic
+    # ports are explicitly hidden, this is an internal service only.


ceejbot · 2019-06-08T13:20:07Z

package.json

    "start": "docker-compose up",
-    "test": "for d in cli services/registry; do cd $d; npm t; cd -; done"
+    "test": "for d in cli services/{registry,workers,web,common/boltzmann}; do cd $d; npm t; cd -; done"


This package.json is a makefile, to be quite honest.

ceejbot · 2019-06-08T13:22:30Z

services/common/boltzmann/client/index.js

+    this.logger = logger;
+  }
+
+  async getProviders() {


Chris. have you... have you slept in the last week? I feel super-lazy now.

This is mostly mechanical work, I realize. It makes me understand why people want things like Swagger, because code generation is so tempting. (Resist the urge!)

ceejbot · 2019-06-08T13:26:19Z

services/common/boltzmann/client/index.js

+    `
+        .trim()
+        .split(/\s+/)
+        .join(''),


The lengths you're going to to avoid long lines!

Something in me always twitches when I see this pattern in your code, because my own urges toward tidiness do not see the long lines, but they do see the work that gets repeated every time the function is called.

ceejbot · 2019-06-08T14:27:43Z

services/storage/handlers/users.js

+  return response.json({
+    objects,
+    next: hasNext,
+    prev: hasPrev


which we'll eventually need to punch through to the gateway. unless it is already and I failed to notice

ceejbot · 2019-06-08T14:28:08Z

services/storage/middleware/internal-auth.js

+      // `context.user` return a promise for the currently
+      // authenticated user (if any.)
+      //
+      // TODO: Someday use JWTs, maybe.


Grumpy sound.

ceejbot · 2019-06-08T14:30:41Z

services/storage/models/user.js

@@ -30,6 +30,11 @@ module.exports = class User {
    this.active = active;
  }

+  toJSON() {
+    const { tfa_secret: _0, backup_codes: _1, id: _2, ...meta } = this;


Good choices to omit.

ceejbot · 2019-06-08T14:31:20Z

services/storage/models/token.js

@@ -34,6 +34,11 @@ module.exports = class Token {
    this.active = active;
  }

+  toJSON() {
+    const { user: _0, id: _1, user_id: _2, ...meta } = this;


ceejbot · 2019-06-08T14:40:07Z

services/web/handlers/auth.js

-    context.session.set('banner', 'Successfully deleted 1 token.');
+    context.session.set(
+      'banner',
+      `Successfully deleted ${count} token${count === 1 ? '' : 's'}.`


this is what an acceptable ternary looks like
memorize this chris
don't nest them
no stop don't

* fix(cli): allow command name to be placed after flags * chore(cli): refactor whoami command to shared utils * feat(cli): store username per registry when logging in * feat(cli): split `invitations` cmd into packages/namespaces * feat(cli): use --namespace/--package in invite cmd * feat(cli): add --namespace/--package flags to decline cmd * feat(cli): add `ds accept` * fix(cli): update routing to match new spec from #135

ceejbot reviewed Jun 4, 2019

View reviewed changes

junoatwork mentioned this pull request Jun 5, 2019

Run Scripts: Do we need them? #150

Closed

This was referenced Jun 5, 2019

Website requirements #107

Open

Vacation / Out of GitHub Jun 8 - Jun 10 #160

Closed

chrisdickinson force-pushed the chris/spin-a-web branch from 9da4900 to f239e24 Compare June 6, 2019 23:13

chrisdickinson mentioned this pull request Jun 7, 2019

implement namespaces workflow #113

Open

superhawk610 mentioned this pull request Jun 7, 2019

feat: add routes for namespace creation/deletion #193

Closed

chrisdickinson force-pushed the chris/spin-a-web branch from a298fe4 to 8a06e47 Compare June 7, 2019 06:08

chrisdickinson commented Jun 7, 2019

View reviewed changes

This was referenced Jun 7, 2019

I saw your talk, and very interesting. But why use the same paradigm as before? #201

Closed

Authentication not found #202

Closed

olingern reviewed Jun 8, 2019

View reviewed changes

services/common/boltzmann/client/index.js Outdated Show resolved Hide resolved

olingern reviewed Jun 8, 2019

View reviewed changes

zacanger reviewed Jun 8, 2019

View reviewed changes

services/storage/handlers/packages.js Outdated Show resolved Hide resolved

zacanger reviewed Jun 8, 2019

View reviewed changes

services/storage/handlers/packages.js Show resolved Hide resolved

chrisdickinson added 7 commits June 7, 2019 22:53

lint, add load-bearing semicolons

90fb9d9

lint, yet more substantially

c48caf6

fix: listing memberships and maintainerships works

7fd801b

eslint: quit grouching about empty blocks

f1cf47e

fix: works to my content, for now

1956c80

chore: rename circleci registry->storage

9b1320b

add mkd

94873c8

chrisdickinson marked this pull request as ready for review June 8, 2019 06:48

chrisdickinson added 2 commits June 8, 2019 00:20

cleanup

7a41d8f

fix: send {count} back from token deletion

b72a604

chrisdickinson commented Jun 8, 2019

View reviewed changes

chrisdickinson changed the title ~~feat: WIP slicing www out of registry~~ Split the registry into registry, web, and storage services Jun 8, 2019

ceejbot approved these changes Jun 8, 2019

View reviewed changes

ceejbot mentioned this pull request Jun 8, 2019

switch from url.parse() to new URL() #211

Closed

This was referenced Jun 8, 2019

Consider jwilder/nginx-proxy for local dev #212

Closed

Server starter boilerplate #213

Closed

Move to Remark #214

Closed

ceejbot merged commit 0be1a86 into master Jun 9, 2019

ceejbot deleted the chris/spin-a-web branch June 9, 2019 14:51

This was referenced Jun 9, 2019

get the cli caught up with pr #135 #217

Open

audit registry dependencies #218

Closed

gribnoysup mentioned this pull request Jun 9, 2019

fix(services/common/boltzmann): Change body.error to body.message #210

Closed

This was referenced Jun 9, 2019

Dockerize new services #220

Merged

feat(ops): Kuberenetes #204

Closed

olingern mentioned this pull request Jun 9, 2019

[CI] - Add lint checking #149

Closed

ceejbot mentioned this pull request Jun 10, 2019

feat(cli): refactor invite/invitations/accept/decline commands #188

Merged

7 tasks

This was referenced Jun 10, 2019

[CLI] - Catch cli up Pt. 1 #234

Merged

Replace mocha with ava; Set basic mocking/stubbing tools for testing; Add a couple of tests #118

Closed


		context.session.set('banner', 'Successfully deleted 1 token.');
		context.session.set('banner', `Successfully deleted ${count} token${count === 1 ? '' : 's'}.`);

		@@ -1,5 +1,7 @@
		'use strict';

		const { parse } = require('url');

		getObject({algo, digest}) // promise for a stream


		response.rawjson(str) // for enemies

Split the registry into registry, web, and storage services #135

Split the registry into registry, web, and storage services #135

Conversation

chrisdickinson commented Jun 4, 2019 • 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

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

chrisdickinson commented Jun 7, 2019 • 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

olingern commented Jun 8, 2019

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

chrisdickinson commented Jun 8, 2019

URL CHANGES

why is it important to move all this cheese now

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

ceejbot 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

chrisdickinson commented Jun 4, 2019 •

edited

Loading

chrisdickinson commented Jun 7, 2019 •

edited

Loading