Another go at --- Map workspace to iprojectsite #3564

KevinRansom · 2017-09-09T08:17:32Z

This supercedes: #3425

It integrates the F# Editor with dotnet sdk project files and gets the file ordering right, correct references, defines and all of that malarkey.

Once more I fully understand, what it's like to be the stupid guy in the room. I have spent forever on this, barely understanding what was going on. However, the solution was a few trivial bug fixes and the realization that FCS gets it's list of references from the command line options.

Anyway ...
I'm afraid you won't be able to try it out yet, since it is dependent on a dotnet project-system change.
This change uses the design time build to get the command line options and notify a MEF import supplied by the F# editor.

I think that fix will be in preview soon, and when that happens nightlies will light this up, and we can get to work on the 10 thousand or so bugs left to be discovered.

I have tried not break the integration for desktop projects, I would appreciate, it if you can try and find out any breaks I introduced.

@tannergooding has been working on fixing the CPS ordering issue, so now the solution explorer can render F# project source files in the correct order ... again you will have to wait for a preview.

The CPS team have fixed an ordering issue that caused them to jumble up the order of project settings when imported, @davkean is working on hooking up Tanner's work with solution explorer and the CPS in-jumbling work.

Enabling all of this was about 10 times more coordination than I expected, but it is slowwwwwwwly coming together.

As you can see my changes have been surprisingly economical. I am a very slow typist, and even slower thinker :-)

@cartermp, @dsyme, @brettfo, @Pilchie
Can you try this out ...

Install from \cpvsbuild\drops\vs\vsuml for a build after yesterday--- it has the project-system change
Then build and install the open source vsix.

cartermp · 2017-09-09T20:56:30Z

Looks like this fails to compile with a few things in FSharp.Editor

Pilchie

Nice!

Pilchie · 2017-09-09T23:02:33Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

    let projectTable = ConcurrentDictionary<ProjectId, Refreshable<ProjectId[] * FSharpProjectOptions>>()

    // A table of information about single-file projects.  Currently we only need the load time of each such file, plus
    // the original options for editing
    let singleFileProjectTable = ConcurrentDictionary<ProjectId, DateTime * FSharpProjectOptions>()

+    // Accumulate sorces and references for each project file


Typo - sorces

Pilchie · 2017-09-09T23:03:09Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

    /// Clear a project from the project table
    member this.ClearInfoForProject(projectId: ProjectId) =
        projectTable.TryRemove(projectId) |> ignore
        this.RefreshInfoForProjectsThatReferenceThisProject(projectId)
+        //@@@@@@@ sourceFiles / referenceFiles / commandlineOptions


@@@@@@ is a mnemonic to me that I need to review this before I commit. It is an incredibly old habit of mine. When I say old, I mean from the very early 80's.

saul · 2017-09-10T09:49:19Z

@tannergooding has been working on fixing the CPS ordering issue, so now the solution explorer can render F# project source files in the correct order ... again you will have to wait for a preview.

I thought we were going to try having a separate node for 'Compile Order', so then we didn't have to have completely unique Solution Explorer project node rendering code? Then we don't have to mess about with tricky folder rendering logic and the like. I just don't think it's worth the dev effort to implement it correctly (difficult) and then to maintain it forever. Not to mention we'd be diverging from C#'s project node rendering code, which means we don't get bug fixes/new features that they implement for free.

dsyme · 2017-09-10T15:43:33Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

@@ -32,6 +35,11 @@ open Microsoft.VisualStudio.Shell.Interop
 open Microsoft.VisualStudio.FSharp.LanguageService
 open Microsoft.VisualStudio.ComponentModelHost

+module LogFile =


Please use System.Diagnostics.Trace - no more logging mechanisms please :)

@dsyme It won't get merged, I promise.

dsyme · 2017-09-10T15:44:09Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

-                    Assert.Exception(ex)
-                } |> Async.StartImmediate
-            )
+                async {


Looks like there has been some reformatting of code here - best to undo that for this PR I think

dsyme · 2017-09-10T15:44:32Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

-[<Export(typeof<FSharpProjectOptionsManager>); Composition.Shared>]
-type internal FSharpProjectOptionsManager 
+[<Export(typeof<ProjectInfoManager>); Composition.Shared>]
+type internal ProjectInfoManager 


You might want to undo this renaming, to make the PR really minimal.

(Looking further - yes, please undo the renaming for this PR so we can check it carefully, thanks)

@dsyme ... I'm good with the rename. It's back to what it originally was :-)

I'm fine with the rename, I just wanted the diff minimized (i.e. do it in a separate PR)

I know I'm sounding like a broken record, but the diff would be much smaller without this rename. LanguageService.fs is a PITA and we need to really scrutinize and minimize all changes to it, and potentially undo them if we get them wrong.

dsyme · 2017-09-10T15:50:49Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

+
+        {new IProvideProjectSite with
+            member iProvideProjectSite.GetProjectSite() =
+                let sources () = 


I think I mentioned this in a previous review - but it is clearer without these micro-functions, e.g.

{new IProvideProjectSite with member iProvideProjectSite.GetProjectSite() = let targetFrameworkMoniker = "" let creationTime = System.DateTime.Now let mutable errorReporter = let reporter = ProjectExternalErrorReporter(project.Id, "FS", this.SystemServiceProvider) Some(reporter:> Microsoft.VisualStudio.Shell.Interop.IVsLanguageServiceBuildErrorReporter2) {new Microsoft.VisualStudio.FSharp.LanguageService.IProjectSite with member __.SourceFilesOnDisk() = projectInfoManager.GetProjectInfo(project.FilePath) |> p13 member __.DescriptionOfProject() = project.Name member __.CompilerFlags() = let _,references,options = projectInfoManager.GetProjectInfo(project.FilePath) Array.concat [options; references |> Array.map(fun r -> "-r:" + r)] member __.ProjectFileName() = project.FilePath member __.AdviseProjectSiteChanges(_,_) = () member __.AdviseProjectSiteCleaned(_,_) = () member __.AdviseProjectSiteClosed(_,_) = () member __.IsIncompleteTypeCheckEnvironment = false member __.TargetFrameworkMoniker = targetFrameworkMoniker member __.ProjectGuid = project.Id.Id.ToString() member __.LoadTime = creationTime member __.ProjectProvider = Some iProvideProjectSite member __.AssemblyReferences() = projectInfoManager.GetProjectInfo(project.FilePath) |> p33 member __.BuildErrorReporter with get () = errorReporter and set (v) = errorReporter <- v }

dsyme · 2017-09-10T15:52:53Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

+                                                      set (v) = errorReporter <- v
+                }
+
+        interface IVsHierarchy with


I think I mentioned this in a previous review, but please document why IVsHierarchy is required here. I think it is because of whackiness in ProjectSitesAndFiles.fs where we query for this interface.

dsyme · 2017-09-10T15:54:40Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

                    this.SetupProjectFile(siteProvider, this.Workspace, "SetupNewTextView")
-                | _ -> 
+                | _ when not (IsScript(filename)) ->


When and why would hier not implement IProvideProjectSite? Please document this, it is some case that is not intuitive.

CPS loaded projects don't support IProvideProjectSite. Which is why we have to do this wrapping thingy.

dsyme · 2017-09-10T16:01:55Z

vsintegration/src/FSharp.LanguageService/ProjectSitesAndFiles.fs

@@ -182,7 +184,7 @@ type internal ProjectSitesAndFiles() =
             UseScriptResolutionRules = SourceFile.MustBeSingleFileProject fileName
             LoadTime = projectSite.LoadTime
             UnresolvedReferences = None
-             OriginalLoadReferences = []
+             OriginalLoadReferences =  (projectSite.AssemblyReferences() |> Array.map(fun s -> rangeCmdArgs, s) |> Array.toList)


This doesn't look right. OriginalLoadReferences doesn't contain assembly references - it contains #load references. It should be renamed to OriginalHashLoadDIrectives or something. In more detail:

The comment on the declaration of OriginalLoadReferences is Unused in this API.

In reality, it seems it is populated only for scripts, i.e. OriginalLoadReferences = loadClosure.OriginalLoadReferences. Its purpose is to make sure the incremental builder for script checking gets re-created if the #load change in any way.

You seem to be trying to reuse it to propagate the assembly references some how, including for projects, I'm not quite sure how or if it's essential to the PR

@dsyme yes, I agree ... it is from when I first did this, I will fix it before I commit.

That's ok, just document it please :)

No ... I will remove it, it is not necessary. And I was trying to do exactly what you thought. references are currently propagated using the command line options ... for better or worse.

cartermp · 2017-09-10T17:22:35Z

@saul That was actually our preferred option (and original plan), but we came to the conclusion that this wouldn't be possible after meeting with the CPS team and figuring out how to implement a proper design that doesn't result in a horrible F# experience or break others.

We're punting folder support for after 15.5, rather than hack something in which will end up requiring a rewrite. The CPS team is adding the low-level support for folders which respect ordering, but it won't land in the 15.5 update given the work they already had to do to support F#. The design @tannergooding implemented is flexible for when that work is done.

KevinRansom · 2017-09-11T17:46:13Z

@saul, that is not part of this work. If I recall the conversations, there may need to be additional CPS work to make that work correctly.

dsyme · 2017-09-11T19:30:21Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

-                let targetFrameworkMoniker = ""
-                let projectGuid () = project.Id.Id.ToString()
-                let creationTime = System.DateTime.Now
+                let fst  (a, _, _) = a


Please call these p13 and p33 - that's what they are called in illib.fs of the F# compiler. You can open that module if you like though perhaps best not to

dsyme · 2017-09-11T19:37:58Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

+    member private this.OnProjectChanged(projectId:ProjectId, _newSolution:Solution) =
+        let project = this.Workspace.CurrentSolution.GetProject(projectId)
+        let siteProvider = this.ProvideProjectSiteProvider(this.Workspace, project)
+        this.SetupProjectFile(siteProvider, this.Workspace, "SetupNewTextView")


Have you tested add/remove file, add/remove reference and other changes to the project?

I don't understand what happens as the project changes. It seems that SetupProjectFile gets called multiple times with a different siteProvider each time. But this feels wrong - SetupProjectFile is really only intended to be called once for each project.

Please change "SetupNewTextView" to "OnProjectChanged" (this text appears in FCS reactor trace diagnostics indicating the causal chain for operations int he FCS event queue)

dsyme · 2017-09-11T19:39:20Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

+        let siteProvider = this.ProvideProjectSiteProvider(this.Workspace, project)
+        this.SetupProjectFile(siteProvider, this.Workspace, "SetupNewTextView")
+    member private this.OnProjectAdded(projectId:ProjectId, newSolution:Solution) = this.OnProjectChanged(projectId, newSolution)
+    member private this.OnProjectRemoved(_projectId:ProjectId, _newSolution:Solution) = ()


Why don't we do anything when the project is removed?

dsyme · 2017-09-11T19:40:50Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

        this.Workspace.Options <- this.Workspace.Options.WithChangedOption(Completion.CompletionOptions.BlockForCompletionItems, FSharpConstants.FSharpLanguageName, false)
        this.Workspace.Options <- this.Workspace.Options.WithChangedOption(Shared.Options.ServiceFeatureOnOffOptions.ClosedFileDiagnostic, FSharpConstants.FSharpLanguageName, Nullable false)
+        this.Workspace.WorkspaceChanged.Add(workspaceChanged)
+        this.Workspace.DocumentClosed.Add <| fun args -> tryRemoveSingleFileProject args.Document.Project.Id


Are there any other events we should be listening into here?

Almost certainly document added/removed and changed but I think we should get a grip on performance first.

You probably don't need DocumentTextChanged (as that's handled elsewhere), but maybe the others (for file renames, etc).

dsyme · 2017-09-11T19:46:19Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

            let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
+
+            projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace, userOpName)


This call has been moved from under the if ... then below. I believe this causes it to be executed many more times in a DAG graph of projects + project references, since we recursively call setup for each referenced project.

But what's the specific reason to make this change? Is it because you call SetupProjectFile again for each change in the CPS project (see comment above)? In that case, it feels like you should only be calling UpdateProjectInfo in the incremental case for a change to a CPS project - the rest of SetupProjectFile isn't needed

There's an exception to that - the calls to AddProjectReference are only being made in SetupProjectFile - but that is addressed in the as-yet-unmerged #3499 (with which this will have conflicts I think - hence trying to minimize the diff)

dsyme · 2017-09-11T19:49:12Z

Given this comment it's important to please check that solution load times have not regressed, e.g. please check the devenv.exe-CPU-time-to-first-availability-of-intellisense for tests\projects\stress\big\dense\Dense.sln

Also given the likelihood of conflict with #3499 it would indeed be really great to completely minimize the diff - thanks

KevinRansom · 2017-09-11T20:01:14Z

@dsyme ... project references are a bit broken in this. I need to look at them. I will fix up any renames that are necessary.

KevinRansom · 2017-09-11T20:02:47Z

@dsyme ...
I am amazed I got anything working at all ... for the longest time I was pounding around in the dark.

cartermp · 2017-09-12T01:39:49Z

I cannot build the tools against vsuml, no matter what I try. @KevinRansom what is your exact setup?

KevinRansom · 2017-09-12T01:54:55Z

@cartermp.

In my case, I have both vs2017.3/vsuml sideby side on the one box.
Build in vs2017.3 dos box. Select to vsuml when installing vsix.

Kevin

cartermp · 2017-09-12T01:59:10Z

Ah, that would do it. I'll give it a go; otherwise, I'll be in office tomorrow and I can hijack your box for an hour or so.

KevinRansom · 2017-09-12T02:04:40Z

I need you to find the bugs ... so hijacking my box is no good :-)

KevinRansom · 2017-09-13T21:34:03Z

@dsyme, @cartermp ...
so ... adding async to UpdateProjectInfo makes Loading the VisualFSharp solution super zippy.

On my MacBook parallels VM, 15.4 loads VisualFSharp.sln in around 1 minute, 30 seconds. With the netsdk and all of the update events, I quit timing after 12 minutes.

With this async change it loads in around 30 seconds. woot! woot!

Take a look, and let's see what this breaks.

Thx

Kevin

Pilchie · 2017-09-13T21:46:19Z

Presumably the projects are not created, and IntelliSense doesn't work for that 12+ minutes...

KevinRansom · 2017-09-13T22:06:31Z

@Pilchie no intellisense ... indeed.

cartermp · 2017-09-14T00:36:45Z

@KevinRansom nothing seems broken on initial testing, but I did notice that IntelliSense was a little slow to pop up on a brand-new file. This is with the debug VSIX though, so it could just be that.

dsyme

Looking much simpler - but take the addition Async.Start to a separate issue, and I don't understand the need to add an event trigger

dsyme · 2017-09-14T10:58:43Z

src/fsharp/vs/IncrementalBuild.fs

@@ -1725,12 +1743,16 @@ type IncrementalBuilder(tcGlobals,frameworkTcImports, nonFrameworkAssemblyInputs

                // Never open PDB files for the language service, even if --standalone is specified
                tcConfigB.openDebugInformationForLaterStaticLinking <- false
-
+                match commandLineArgs |> Seq.tryFindIndex(fun s -> s.StartsWith(targetProfileSwitch)) with


Minor nit - It's better to use Seq.tryFind, but it's no big deal

dsyme · 2017-09-14T11:09:45Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

+        let sourcePaths = sources |> Seq.map(fun s -> fullPath s.Path) |> Seq.toArray
+        let referencePaths = references |> Seq.map(fun r -> fullPath r.Reference) |> Seq.toArray
+        projectInfo.[path] <- (sourcePaths,referencePaths,options.ToArray())
+        notify.Trigger(path)


This all seems contorted, making event spaghetti where it's not needed. You have

NewProjectSystem --> calls ProjectOptionsManager (HandleCommandLineChange) --> notify FSharpLanguageService via event --> calls ProjectOptionsManager (UpdateProjectInfo)

Better would surely be

NewProjectSystem --> calls FSharpLanguageService (HandleCommandLineChange) --> calls ProjectOptionsManager (UpdateProjectInfo)

With no need for events to implement what is basically a call.

dsyme · 2017-09-14T11:11:13Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

@@ -294,17 +315,19 @@ and
    [<ProvideEditorExtension(FSharpConstants.editorFactoryGuidString, ".fsscript", 97)>]
    [<ProvideEditorExtension(FSharpConstants.editorFactoryGuidString, ".ml", 97)>]
    [<ProvideEditorExtension(FSharpConstants.editorFactoryGuidString, ".mli", 97)>]
-    internal FSharpLanguageService(package : FSharpPackage) =
+    internal FSharpLanguageService(package : FSharpPackage)  as this =


Using as this is a code smell, and I believe it's only needed because of the contorted use of an event above?

dsyme · 2017-09-14T11:13:16Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

-                projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace, userOpName)
-                let projectContextFactory = package.ComponentModel.GetService<IWorkspaceProjectContextFactory>();
-                let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider)
+            projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace, userOpName)


I still don't understand why there are any changes in this method, which is only used for legacy project files - especially the change that lifts UpdateProjectInfo from under the conditional. I would be more comfortable if changes in this method were completely reverted

dsyme · 2017-09-14T11:15:51Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

+                let referencedProjectIds = referencedProjects |> Array.choose tryGetOrCreateProjectId
+                checkerProvider.Checker.InvalidateConfiguration(options, startBackgroundCompileIfAlreadySeen = not isRefresh, userOpName= userOpName + ".UpdateProjectInfo")
+                referencedProjectIds, options))
+        } |> Async.Start


This looks like an orthogonal fix. We need to think about this carefully - the computation of the options may now terminate in different orders, for example, with update A terminating after update B.

I think it's best to put this in a separate PR.

KevinRansom · 2017-09-14T21:53:53Z

@dsyme, quite a big refactor ... I think I covered all of the feedback, please take a look.

…ntime

dsyme · 2017-09-15T09:56:53Z

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

            let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)

-            if isNull (workspace.ProjectTracker.GetProject projectId) then


I still don't understand why there are any changes in this method, which is only used for old-style project files Why not just revert them all?

dsyme

All ok, except please revert the changes in SetupProjectFile fully

cartermp · 2017-09-15T23:13:16Z

FYI: cloning this repo ---> open ClientServer.sln in VS all works. Projects opened, restore kicks off, and after it's finished the language service kicks in and everything works. Semantic colors, IntelliSense, etc.

cartermp · 2017-09-15T23:21:23Z

Scripting also seems fine:

cartermp

Experience-wise this is good. Only issues I noticed where the file tree problems (which has work that is incoming anyways), and the project references. Let's tackle the latter in another PR.

KevinRansom · 2017-09-15T23:39:18Z

@cartermp project -project references doesn't work for dotnet sdk projects

cartermp · 2017-09-15T23:51:29Z

Yes, I'm fully aware. We agreed to take that with a seoarate PR if I'm not mistaken.

KevinRansom · 2017-09-15T23:52:47Z

I'm not sure if we told anyone in the community. :-)

Pilchie · 2017-09-15T23:55:21Z

Regardless, if the rest of it is solid, I'd like to get this merged so we can make sure to get it in 15.5 P1. We can continue to make progress after that though.

KevinRansom · 2017-09-16T00:21:39Z

@Pilchie It's going in as soon as it's passed the ci.

KevinRansom · 2017-09-16T00:22:17Z

Which is now :-)

* Saved * SDK editor support * Address merge issues * Feedback * reduce delta * invalidate ide when a new file is discovered * Go faster stripes * feedback + refactor * tryFindIndex * Cleanup + Ensure that FSharp.Core.BuildFromSource.proj works at designtime * final clean up * label for HandleCommandLineChanges * ensure some buildfromsource porojects load

KevinRansom added 3 commits August 31, 2017 15:44

Saved

024a45e

SDK editor support

ad48193

merge master

2429048

msftclas added the cla-already-signed label Sep 9, 2017

KevinRansom mentioned this pull request Sep 9, 2017

Map workspace to iprojectsite #3425

Closed

Pilchie approved these changes Sep 9, 2017

View reviewed changes

dsyme requested changes Sep 10, 2017

View reviewed changes

KevinRansom added 2 commits September 11, 2017 11:16

Address merge issues

62e24ca

Feedback

3009567

dsyme reviewed Sep 11, 2017

View reviewed changes

Go faster stripes

8164c61

dsyme requested changes Sep 14, 2017

View reviewed changes

KevinRansom added 2 commits September 14, 2017 14:28

feedback + refactor

418e214

tryFindIndex

f8635f0

Cleanup + Ensure that FSharp.Core.BuildFromSource.proj works at desig…

3fac6c6

…ntime

dsyme reviewed Sep 15, 2017

View reviewed changes

dsyme requested changes Sep 15, 2017

View reviewed changes

KevinRansom added 3 commits September 15, 2017 15:24

final clean up

ef50517

label for HandleCommandLineChanges

b5c5fa9

ensure some buildfromsource porojects load

f0c5734

cartermp approved these changes Sep 15, 2017

View reviewed changes

KevinRansom merged commit 602daf8 into dotnet:master Sep 16, 2017

KevinRansom deleted the editor branch September 16, 2017 00:22

cartermp mentioned this pull request Sep 16, 2017

Visual Studio and Rider cannot deal with Fable projects fable-compiler/Fable#1151

Closed

KevinRansom mentioned this pull request Sep 20, 2017

15.3 preview 3: FCS does not respect file ordering defined in .NET Core project files #3260

Closed

cartermp mentioned this pull request Oct 2, 2017

[From 15.5 preview test day] - Newly-added file is not recognized #3677

Closed

		let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)

		projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace, userOpName)

		let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)

		if isNull (workspace.ProjectTracker.GetProject projectId) then

Another go at --- Map workspace to iprojectsite #3564

Another go at --- Map workspace to iprojectsite #3564

Conversation

KevinRansom commented Sep 9, 2017 • edited by cartermp Loading

cartermp commented Sep 9, 2017

Pilchie 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

saul commented Sep 10, 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

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

cartermp commented Sep 10, 2017 • edited Loading

KevinRansom commented Sep 11, 2017

dsyme Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

dsyme Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

dsyme Sep 11, 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

dsyme Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

dsyme commented Sep 11, 2017 • edited Loading

KevinRansom commented Sep 11, 2017

KevinRansom commented Sep 11, 2017

cartermp commented Sep 12, 2017

KevinRansom commented Sep 12, 2017

cartermp commented Sep 12, 2017

KevinRansom commented Sep 12, 2017

KevinRansom commented Sep 13, 2017

Pilchie commented Sep 13, 2017

KevinRansom commented Sep 13, 2017

cartermp commented Sep 14, 2017 • edited Loading

dsyme 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

KevinRansom commented Sep 14, 2017

Choose a reason for hiding this comment

dsyme left a comment

Choose a reason for hiding this comment

cartermp commented Sep 15, 2017

cartermp commented Sep 15, 2017

cartermp left a comment

Choose a reason for hiding this comment

KevinRansom commented Sep 15, 2017

cartermp commented Sep 15, 2017

KevinRansom commented Sep 15, 2017

Pilchie commented Sep 15, 2017

KevinRansom commented Sep 16, 2017

KevinRansom commented Sep 16, 2017

KevinRansom commented Sep 9, 2017 •

edited by cartermp

Loading

cartermp commented Sep 10, 2017 •

edited

Loading

dsyme Sep 11, 2017 •

edited

Loading

dsyme Sep 11, 2017 •

edited

Loading

dsyme Sep 11, 2017 •

edited

Loading

dsyme Sep 11, 2017 •

edited

Loading

dsyme commented Sep 11, 2017 •

edited

Loading

cartermp commented Sep 14, 2017 •

edited

Loading