RFC to build fastboot in ember-cli. #77

kratiahuja · 2016-11-11T01:11:38Z

rendered

Corresponding PR which does the work described in this RFC is here.

Long term RFC which should allow this to become a generic feature is described in this RFC

cc: @stefanpenner @tomdale @rwjblue @ryanone @masonwan @kristoferbaxter @arjansingh

kratiahuja · 2016-11-11T01:20:22Z

Some history are mentioned in the following issues:

rwjblue

Notes:

Various references to fastboot should be "FastBoot".
Is there a specific reason to rely on the "last module registered wins" clobbering behavior as opposed to relying on broccoli-merge-trees (with { overwrite: true }) to allow only the final concat to differ?
I would have thought that we would want a browser/app and browser/addon. This way the main generic app bundles live in addon or app and the "platform specific" parts live in either browser/ or fastboot/. The resulting build would have app.js, browser-app.js, fastboot-app.js, and possibly a combined-app.js (which would be the main app bundle along with the main browser bundle).

rwjblue · 2016-11-11T04:28:53Z

active/0000-build-fastboot-in-ember-cli.md

+
+This RFC will primarily allow fastboot assets to be built using `ember build`. Fastboot is a different environment in which your ember apps run and require your app behavior to change a bit in this environment that running your app in the browser. This requires building fastboot assets that compliment the current browser assets.
+
+This RFC describes the details of how fastboot will be built inside `ember-cli`. Since we do not know the generic usecases, we would like to build this `ember-cli` as the first pass. Once we have understood the usecases, we can make it generic per this [RFC](https://github.com/ember-cli/rfcs/pull/75). This RFC aims to expose the minimal public API and work to build fastboot in `ember-cli`.


would like to build this ember-cli as the first pass

Sounds a bit odd, maybe you mean:

would like to build this into ember-cli as the first pass

?

rwjblue · 2016-11-11T04:31:11Z

active/0000-build-fastboot-in-ember-cli.md

+This RFC describes the details of how fastboot will be built inside `ember-cli`. Since we do not know the generic usecases, we would like to build this `ember-cli` as the first pass. Once we have understood the usecases, we can make it generic per this [RFC](https://github.com/ember-cli/rfcs/pull/75). This RFC aims to expose the minimal public API and work to build fastboot in `ember-cli`.
+
+# Motivation
+[Fastboot](https://ember-fastboot.com) allows ember apps to be rendered on the server side. It renders the initial content of your app allowing your app to be used in SEO usecases. On a very high level, fastboot loads your assets in a Node environment and creates the Ember App once the assets are loaded. It does not do any automatic routing but creates Ember application instance whenever there is a server side rendering request and runs your route to render the initial render in Node.


It does not do any automatic routing but creates Ember application instance whenever there is a server side rendering request and runs your route to render the initial render in Node.

Slight tweak for correctness:

It does not do any automatic routing but creates an Ember.ApplicationInstance instance whenever there is a server side rendering request and runs your applications initial render in Node.

rwjblue · 2016-11-11T04:32:30Z

active/0000-build-fastboot-in-ember-cli.md

+# Motivation
+[Fastboot](https://ember-fastboot.com) allows ember apps to be rendered on the server side. It renders the initial content of your app allowing your app to be used in SEO usecases. On a very high level, fastboot loads your assets in a Node environment and creates the Ember App once the assets are loaded. It does not do any automatic routing but creates Ember application instance whenever there is a server side rendering request and runs your route to render the initial render in Node.
+
+In order for your app/addon to be able to run in a Node environment, it needs to be compatible in such environment. Compatibility includes not doing DOM/window access in Node, making sure you are able to make API requests in Node etc. Therefore an app/addon may need to provide fastboot specific initializers, instance-initializers, services etc that are only applicable in the Node environment and not in the browser. Including them in the browser has performance implication since you will send down more bytes to the client.


Including them in the browser has performance implication since you will send down more bytes to the client.

Might read better as:

Including them in the browser has performance implications since you would be sending down more bytes than required to the client at runtime.

rwjblue · 2016-11-11T04:33:26Z

active/0000-build-fastboot-in-ember-cli.md

+
+In order for your app/addon to be able to run in a Node environment, it needs to be compatible in such environment. Compatibility includes not doing DOM/window access in Node, making sure you are able to make API requests in Node etc. Therefore an app/addon may need to provide fastboot specific initializers, instance-initializers, services etc that are only applicable in the Node environment and not in the browser. Including them in the browser has performance implication since you will send down more bytes to the client.
+
+Currently fastboot builds the app twice and generates a different set of assets for fastboot and browser environments via ember fastboot command. Most of the app between the two sets of assets is same except where we want fastboot to override the browser behavior or need fastboot specific handling. Currently ember fastboot filters these and generates the two sets of assets. Building different sets of assets (app and vendor files) for different environments cannot guarantee the app works correctly in both environments always. Moreover, ember-fastboot --serve-assets command also spins up a node server and serves your assets. This is very similar to what ember serve does today. However, we end up forking and creating two parallel worlds with problems such as serving stale assets, creating race conditions during serving and building etc. We want to be able converge the two worlds such that fastboot should be able to specify the last middleware that will be used to serve the assets in fastboot. This helps us not create fastboot commands and keeps the eco-system unified.


ember-fastboot --serve-assets command

Should be:

ember fastboot --serve-assets command

rwjblue · 2016-11-11T04:35:25Z

active/0000-build-fastboot-in-ember-cli.md

+
+In order for your app/addon to be able to run in a Node environment, it needs to be compatible in such environment. Compatibility includes not doing DOM/window access in Node, making sure you are able to make API requests in Node etc. Therefore an app/addon may need to provide fastboot specific initializers, instance-initializers, services etc that are only applicable in the Node environment and not in the browser. Including them in the browser has performance implication since you will send down more bytes to the client.
+
+Currently fastboot builds the app twice and generates a different set of assets for fastboot and browser environments via ember fastboot command. Most of the app between the two sets of assets is same except where we want fastboot to override the browser behavior or need fastboot specific handling. Currently ember fastboot filters these and generates the two sets of assets. Building different sets of assets (app and vendor files) for different environments cannot guarantee the app works correctly in both environments always. Moreover, ember-fastboot --serve-assets command also spins up a node server and serves your assets. This is very similar to what ember serve does today. However, we end up forking and creating two parallel worlds with problems such as serving stale assets, creating race conditions during serving and building etc. We want to be able converge the two worlds such that fastboot should be able to specify the last middleware that will be used to serve the assets in fastboot. This helps us not create fastboot commands and keeps the eco-system unified.


However, we end up forking and creating two parallel worlds with problems such as serving stale assets, creating race conditions during serving and building etc.

Should also mention 2X build speed issues

We want to be able converge the two worlds such that fastboot should be able to specify the last middleware that will be used to serve the assets in fastboot.

This is slightly confusing to me (from the RFC's perspective "what are middlewares?", "why do I care about the last middleware?", etc), can you tweak a bit to make it clearer?

This helps us not create fastboot commands and keeps the eco-system unified.

We should explain why we care about not duplicating commands and whatnot...

Fixed explaining the double builds. Apologies the middleware shouldn't have been part of this RFC. That will likely be a different RFC (hopefully coming next week...)

rwjblue · 2016-11-11T04:49:11Z

active/0000-build-fastboot-in-ember-cli.md

+
+## Public API
+With building the additional assets, this RFC would also like to expose some public APIs so that addons that are doing fastboot specific things can leverage that to build fastboot assets. The additional public API will be as follows:
+1. `preprocessTree(type, tree)`: Currently the type includes only for browser specific assets. We want to invoke this hook for fastboot asset as well. Therefore, when fastboot assets are built, this hook will be invoked with type as `fastboot`. Addons can use this hook to munge the fastboot/app tree before it gets processed.


this hook will be invoked with type as fastboot

This should be fastboot-app and fastboot-addon

Fixed to use app-fastboot and dropped the fastboot-addon per the scope of this RFC.

rwjblue · 2016-11-11T04:49:32Z

active/0000-build-fastboot-in-ember-cli.md

+## Public API
+With building the additional assets, this RFC would also like to expose some public APIs so that addons that are doing fastboot specific things can leverage that to build fastboot assets. The additional public API will be as follows:
+1. `preprocessTree(type, tree)`: Currently the type includes only for browser specific assets. We want to invoke this hook for fastboot asset as well. Therefore, when fastboot assets are built, this hook will be invoked with type as `fastboot`. Addons can use this hook to munge the fastboot/app tree before it gets processed.
+2. `postprocessTree(type, tree)`: This currently is also only invoked for browser specific assets. We want to invoke the same hook with `fastboot` as the type when building fastboot asset.


Same as above, should be app-fastboot and addon-fastboot

fastboot-app (as stated above), or app-fastboot? It seems that fastboot-app is more appropriate given how the mapping currently looks:

{ app: 'treeForApp', styles: 'treeForStyles', templates: 'treeForTemplates', 'addon-templates': 'treeForAddonTemplates', 'fastboot-app': 'treeForFastbootApp' // ...? }

@ryanone I think the confusion is from two aspects:

The generated fastboot asset is called app-fastboot.js

The hook is call fastboot.

The reason Rob is suggesting to keep the names same across both the same.

We discussed this. fastboot/addon will not be built as part of this RFC.

rwjblue · 2016-11-11T04:49:55Z

active/0000-build-fastboot-in-ember-cli.md

+With building the additional assets, this RFC would also like to expose some public APIs so that addons that are doing fastboot specific things can leverage that to build fastboot assets. The additional public API will be as follows:
+1. `preprocessTree(type, tree)`: Currently the type includes only for browser specific assets. We want to invoke this hook for fastboot asset as well. Therefore, when fastboot assets are built, this hook will be invoked with type as `fastboot`. Addons can use this hook to munge the fastboot/app tree before it gets processed.
+2. `postprocessTree(type, tree)`: This currently is also only invoked for browser specific assets. We want to invoke the same hook with `fastboot` as the type when building fastboot asset.
+3. `treeForFastbootApp(tree)`: This is a new hook that this RFC proposes to add which can be used by addons to munge the tree for `fastboot/app` before it is processed.


Should also include a treeForFastbootAddon hook

@rwjblue I do not want to add building fastboot/addon since there is no usecase today. In future, if we have a usecase I would agree we want to expose that hook. To satisfy the long term needs (which are undetermined right now), I can add this and about building fastboot/addon but we will not be addressing it in the corresponding PR. Thoughts?

Sounds good to me. We can make a more focused RFC for specifically adding this at a later date.

Sounds good. I am going to drop this then.

rwjblue · 2016-11-11T04:50:21Z

active/0000-build-fastboot-in-ember-cli.md

+
+# How We Teach This
+
+This is an advanced feature and any app that defines `fastboot/app` and runs `ember build` will end up generating an additional asset as `app-fastboot.js`. We would need to update the `ember-cli` documentation to explain users how this additional asset is generated and how it will be of limited use.


Should include guides / docs in ember-fastboot.com

rwjblue · 2016-11-11T04:51:28Z

active/0000-build-fastboot-in-ember-cli.md

+
+# Drawbacks
+
+The only drawback this has is that it builds fastboot specific code in `ember-cli`. This is only being done since we do not want to come up with a generic solution until we fully know the usecases to make this generic that any additional asset can be built.


Relies on loader.js behavior of "last wins", which is not guaranteed to be true for:

a world where we don't use loader.js (i.e. if we move to rollup built assets)

lazy engines?

@rwjblue lazy-engines are loaded upfront in FastBoot environment. A world where loader.js does not live as well, the eval and ordering of loading the assets in the sandbox will still do the override I believe.

Hmm, the vendor asset that we load in fastboot should still have loader.js, and it is used (AFAIK we do not transpile with other things like rollup in ember-cli-fastboot today so we have to still be relying on loader.js there...).

Yes, there is nothing special done in fastboot. The loader definition and API is exposed in the sandbox.

I was answering the question about the rollup. I honestly don't know what is the plan for that.

kratiahuja · 2016-11-11T19:13:50Z

I would have thought that we would want a browser/app and browser/addon. This way the main generic app bundles live in addon or app and the "platform specific" parts live in either browser/ or fastboot/. The resulting build would have app.js, browser-app.js, fastboot-app.js, and possibly a combined-app.js (which would be the main app bundle along with the main browser bundle).

I agree about this as a "long term" goal (see this RFC). If we want to do this with this RFC, it then it means this is a backward incompatible change in ember-cli. The goal of this RFC was:

Keep ember-cli backward compatible
Do not regress the current ember-cli builds that builds browser specific assets.
Do the minimal things in ember-cli such that FastBoot 1.0 is unblocked.

If everyone agrees to the above scope of the problem, this this is what this RFC is supposed to address.

kratiahuja · 2016-11-11T19:19:48Z

Is there a specific reason to rely on the "last module registered wins" clobbering behavior as opposed to relying on broccoli-merge-trees (with { overwrite: true }) to allow only the final concat to differ?

No specific reason, that is how loader.js works today. We can do the final merge of app.js and app-fastboot.js but this is a behavior I would leave upto the platform to determine. (In case of fastboot, ember-cli-fastboot can do that in the postProcessTree for fastboot type).

kratiahuja · 2016-11-12T00:37:33Z

@rwjblue updated per your feedback.

nathanhammond

Getting this up as early as I can for visibility. As a result not perfectly wordsmithed, not entirely constructive. But I intend these to be things for which we can work toward solutions.

nathanhammond · 2016-11-17T18:48:44Z

active/0000-build-fastboot-in-ember-cli.md

+
+# Summary
+
+This RFC will primarily allow FastBoot assets to be built using `ember build`. FastBoot is a different environment in which your ember apps run and require your app behavior to change a bit in this environment that running your app in the browser. This requires building FastBoot assets that compliment the current browser assets.


s/compliment/complement/

nathanhammond · 2016-11-17T18:52:23Z

active/0000-build-fastboot-in-ember-cli.md

+## Directory Structure for FastBoot app/addons
+In order to build the FastBoot assets which are an addition to the app and vendor files, we want to define a separate directory structure. This will allow FastBoot specific app code to live in its own namespace and does not touch the browser specific app file.
+
+*Note*: Currently today no FastBoot specific code lives in the addon namespace. Hence we do not need `FastBoot/addon` namespace and building of that namespace as part of this RFC.


Should we have a concept of a FatBoot-specific addon? How should this get merged from a child addon which needs to do something different in FastBoot?

I feel like we're missing a concept here that I'm sloppily calling treeForFastboot().

(Seen later in the RFC.)

@nathanhammond There is no usecase currently that requires a addon inside another addon to do FastBoot work specifically. In general, the goal of this RFC is to only address the current FastBoot usecases and a very limited usecases.

I am open to discussions about how to get this working for addon inside addon but the way I see it should just work how app namespace is built for addon inside addon.

nathanhammond · 2016-11-17T18:52:36Z

active/0000-build-fastboot-in-ember-cli.md

+
+## Public API
+With building the additional assets, this RFC would also like to expose some public APIs so that addons that are doing FastBoot specific things can leverage that to build FastBoot assets. The additional public API will be as follows:
+1. `preprocessTree(type, tree)`: Currently the type includes only for browser specific assets. We want to invoke this hook for FastBoot asset as well. Therefore, when FastBoot assets are built, this hook will be invoked with type as `app-fastboot`. Addons can use this hook to munge the FastBoot/app tree before it gets processed.


Newline before.

nathanhammond · 2016-11-17T18:54:25Z

active/0000-build-fastboot-in-ember-cli.md

+After an ember build, the generated assets will be:
+1. vendor.js
+2. app.js
+3. app-fastboot.js


I'm opposed to adding an additional asset and new failure mode here. I'd much rather generate app-fastboot.js which is mergeTrees + overwrite: true here and contains the actual result..

@nathanhammond I would like to leave that behavior upto the platform to decide. If we merge the two files, it requires us to fork the app.js such that it works in browser differently and in Node environments differently . In Node environment it will be a merge of the two files while in browser it will just be the original app.js file. In general we want to move away from forking and generating two different assets for the primary purpose that it doesn't really guarantee consistency in both environments correctly.

I'm a firm believer that in our future story we're going to be doing more and more arbitrary splitting of assets. This has already begun with our Engines story and is only going to get more complicated. In other words, I don't know that I agree with this as a long term goal.

We need to instead do a better job allowing for this behavior to be tested but since it will hopefully primarily be a framework level concern we can probably feel relatively confident that it "just works" most of the time.

nathanhammond · 2016-11-17T18:58:04Z

active/0000-build-fastboot-in-ember-cli.md

+
+## Public API
+With building the additional assets, this RFC would also like to expose some public APIs so that addons that are doing FastBoot specific things can leverage that to build FastBoot assets. The additional public API will be as follows:
+1. `preprocessTree(type, tree)`: Currently the type includes only for browser specific assets. We want to invoke this hook for FastBoot asset as well. Therefore, when FastBoot assets are built, this hook will be invoked with type as `app-fastboot`. Addons can use this hook to munge the FastBoot/app tree before it gets processed.


I'm concerned about this being the place that we want to insert things. preprocessTree and postprocessTree receive broad categories of file types (test, template, js, css), not segments of built files. Segmentation tends to be more in the treeFor space inside of Ember CLI.

@nathanhammond The reason why we need these hooks is we want to patch other things (example dom helper patches based on versions) before we start processing the tree. Currently, ember-cli doesn't really distinguish the segments of built files and file types clearly for this hook. In the long run, I would be happy to address this.

Okay, I consider this to be a shortcoming in Ember CLI that we should address.

nathanhammond · 2016-11-17T19:00:33Z

active/0000-build-fastboot-in-ember-cli.md

+With building the additional assets, this RFC would also like to expose some public APIs so that addons that are doing FastBoot specific things can leverage that to build FastBoot assets. The additional public API will be as follows:
+1. `preprocessTree(type, tree)`: Currently the type includes only for browser specific assets. We want to invoke this hook for FastBoot asset as well. Therefore, when FastBoot assets are built, this hook will be invoked with type as `app-fastboot`. Addons can use this hook to munge the FastBoot/app tree before it gets processed.
+2. `postprocessTree(type, tree)`: This currently is also only invoked for browser specific assets. We want to invoke the same hook with `app-fastboot` as the type when building FastBoot asset.
+3. `treeForFastBootApp(tree)`: This is a new hook that this RFC proposes to add which can be used by addons to munge the tree for `fastboot/app` before it is processed.


How are you expecting this to be invoked at nested addon levels? What order guarantees are you expecting? And since you're expecting a tree as input, what is that tree?

@nathanhammond We don't have a usecase currently that requires this to work for nested addon levels. I would assume the order would be same as what ember-cli behavior is for app namespace.

In my head I had a different mental model when posting this comment, which we discussed today at the meeting. This is one of the open problems with my proposal which we need to solve for.

nathanhammond · 2016-11-17T19:01:35Z

active/0000-build-fastboot-in-ember-cli.md

+
+An alternative [RFC](https://github.com/ember-cli/rfcs/pull/75) which is a more generic solution and long term goal in `ember-cli`.
+
+# Unresolved questions


FastBoot needs to be aware of the engine route map as well in order to properly inline assets in response to the initial request.

@nathanhammond See this issue tracking that already. My current WIP branch of ember-cli-fastboot fixes this somewhat to allow the engine route map to be read and inlined. This is not something specific that needs to be done in ember-cli but in ember-fastboot project.

nathanhammond · 2016-11-17T19:04:01Z

active/0000-build-fastboot-in-ember-cli.md

+
+Following are the drawbacks of this approach:
+- [ ] It builds FastBoot specific code in `ember-cli`. This is only being done since we do not want to come up with a generic solution until we fully know the usecases to make this generic that any additional asset can be built.
+- [ ] This approach of loading additive assets after the main asset heavily depends on the loader.js behavior of "last one" wins always. If this behavior changes in future, this approach will break.


Major confirm, I see this as blocking landing.

How is this blocking landing? I am not sure I understand that. Could you please elaborate a bit more?

I mean that I don't think we should rely on this behavior, and that we should design something (similar to our whiteboarding session) which doesn't rely on this. Both @rwjblue and I had the same feedback without consulting each other.

nathanhammond · 2016-11-17T19:07:07Z

active/0000-build-fastboot-in-ember-cli.md

+│   │-- instance-initializers/
+│   │   |-- error-handler.js
+│-- fastboot/
+│   │-- app/


This approach implies strongly that FastBoot is only additive or clobbering. You propose postprocessTree as a way to get out of that, but I'm thinking that we should come up with a better preprocessor/prostprocessor story that allowed for callbacks at the treeFor level instead of at the "giant collection" level.

This makes it far easier to compose with nested dependencies.

kellyselden · 2016-11-17T19:33:09Z

I read through the spike in ember-cli/ember-cli#6395, and it seems like it is so close to being independent of fastboot, all it needs is a supplied "platform".

preprocessTree could change from receiving "app-fastboot" to "app-[platform]"
postprocessTree could change from receiving "app-fastboot" to "app-[platform]"
treeForFastBootApp could change to treeForAppPlatform with a param "type" of "[platform]"

An app or addon could then supply an array of platforms, via ember-cli-build.js or package.json, and ember-cli would look for folders with those names. Then the rest of this RFC would take effect.

rwjblue · 2019-01-19T19:45:34Z

We are working on closing the ember-cli/rfcs repo in favor of using a single central RFC's repo for everything. This was laid out in https://emberjs.github.io/rfcs/0300-rfc-process-update.html.

Sorry for the troubles, but would you mind reviewing to see if this is still something we need, and if so migrating this over to emberjs/rfcs?

Thank you!

RFC to build fastboot in ember-cli.

d166b8e

kratiahuja mentioned this pull request Nov 11, 2016

Add building targets RFC. #75

Merged

rwjblue reviewed Nov 11, 2016

View reviewed changes

Update per feedback.

768fb7e

nathanhammond reviewed Nov 17, 2016

View reviewed changes

rwjblue closed this Jan 19, 2019


		This RFC will primarily allow fastboot assets to be built using `ember build`. Fastboot is a different environment in which your ember apps run and require your app behavior to change a bit in this environment that running your app in the browser. This requires building fastboot assets that compliment the current browser assets.

		This RFC describes the details of how fastboot will be built inside `ember-cli`. Since we do not know the generic usecases, we would like to build this `ember-cli` as the first pass. Once we have understood the usecases, we can make it generic per this [RFC](https://github.com/ember-cli/rfcs/pull/75). This RFC aims to expose the minimal public API and work to build fastboot in `ember-cli`.


		In order for your app/addon to be able to run in a Node environment, it needs to be compatible in such environment. Compatibility includes not doing DOM/window access in Node, making sure you are able to make API requests in Node etc. Therefore an app/addon may need to provide fastboot specific initializers, instance-initializers, services etc that are only applicable in the Node environment and not in the browser. Including them in the browser has performance implication since you will send down more bytes to the client.

		Currently fastboot builds the app twice and generates a different set of assets for fastboot and browser environments via ember fastboot command. Most of the app between the two sets of assets is same except where we want fastboot to override the browser behavior or need fastboot specific handling. Currently ember fastboot filters these and generates the two sets of assets. Building different sets of assets (app and vendor files) for different environments cannot guarantee the app works correctly in both environments always. Moreover, ember-fastboot --serve-assets command also spins up a node server and serves your assets. This is very similar to what ember serve does today. However, we end up forking and creating two parallel worlds with problems such as serving stale assets, creating race conditions during serving and building etc. We want to be able converge the two worlds such that fastboot should be able to specify the last middleware that will be used to serve the assets in fastboot. This helps us not create fastboot commands and keeps the eco-system unified.


		# How We Teach This

		This is an advanced feature and any app that defines `fastboot/app` and runs `ember build` will end up generating an additional asset as `app-fastboot.js`. We would need to update the `ember-cli` documentation to explain users how this additional asset is generated and how it will be of limited use.


		# Drawbacks

		The only drawback this has is that it builds fastboot specific code in `ember-cli`. This is only being done since we do not want to come up with a generic solution until we fully know the usecases to make this generic that any additional asset can be built.


		# Summary

		This RFC will primarily allow FastBoot assets to be built using `ember build`. FastBoot is a different environment in which your ember apps run and require your app behavior to change a bit in this environment that running your app in the browser. This requires building FastBoot assets that compliment the current browser assets.


		An alternative [RFC](https://github.com/ember-cli/rfcs/pull/75) which is a more generic solution and long term goal in `ember-cli`.

		# Unresolved questions

RFC to build fastboot in ember-cli. #77

RFC to build fastboot in ember-cli. #77

Conversation

kratiahuja commented Nov 11, 2016

kratiahuja commented Nov 11, 2016

rwjblue 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanone Nov 11, 2016 • 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

kratiahuja commented Nov 11, 2016

kratiahuja commented Nov 11, 2016

kratiahuja commented Nov 12, 2016

nathanhammond 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

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

kellyselden commented Nov 17, 2016

rwjblue commented Jan 19, 2019

ryanone Nov 11, 2016 •

edited

Loading