Convert all projects to SDK-style #29831

wtgodbe · 2018-05-21T20:54:01Z

Part of https://github.com/dotnet/corefx/projects/3

CC @weshaggard @joperezr @safern @eerhardt

joperezr · 2018-05-21T22:29:21Z

Did you require any buildtools changes with this? do you mind sharing those if you did?

weshaggard · 2018-05-21T22:34:23Z

src/Directory.Build.props

+  <PropertyGroup>
+    <EnableDefaultItems>false</EnableDefaultItems>
+    <NoWarn>CS3021;$(NoWarn)</NoWarn>
+    <RestoreOutputPath>$(MSBuildThisFileDirectory)\System.Runtime\ref\</RestoreOutputPath>


Why is this going to System.Runtime\ref?

This is a remnant of changes I brought in from Jose's original changeset - the location pointed to here needs a project.assets.json file, which I also added in this change. I'm not married to that particular location

This is wrong, I had it like this as part of my changes only when I was iterating on some changes and wanted to verify that stuff was being restored correctly. This should be removed.

weshaggard · 2018-05-21T22:34:50Z

src/Directory.Build.props

+    <IsSDKProject>true</IsSDKProject>
+  </PropertyGroup>
+  <PropertyGroup>
+    <CSharpCoreTargetsPath>$(MSBuildExtensionsPath32)\Roslyn\Microsoft.CSharp.Core.targets</CSharpCoreTargetsPath>


Why do we need to explicitly set these here?

This is also a remnant I took from Jose's original changeset - I haven't tried running a build without it yet.

weshaggard · 2018-05-21T22:35:24Z

src/Directory.Build.targets

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project InitialTargets="AddSkipGetTargetFrameworkToProjectReferences">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), dir.targets))\dir.targets" />


What are these targets for?

This is just what imports the parent directory's dir.targets. We used to have this per project, and we now have it only once on the common file.

Should we instead of import dir.targets, rename dir.targets to Directory.Build.Targets?

I would be fine with that, though it would probably confuse some devs at first

Directory.Build.Targets is a new feature of msbuild that some repos under dotnet/aspnet have started to use already:
https://github.com/search?q=org:dotnet+filename:directory.build.targets
https://github.com/search?q=org:aspnet+filename:directory.build.targets

Do you think I should rename all dir.props/dir.targets to Directory.Build.props/targets? @joperezr is there documentation on how SDK projects automatically find files named as such? Will there be collisions if multiple files share that name across different parts of the directory tree?

is there documentation on how SDK projects automatically find files named as such?

Check out https://docs.microsoft.com/en-us/visualstudio/msbuild/customize-your-build

dir.props and dir.targets are maltilayered in many repos (they are present at multiple directory levels and recursively each one includes higher level projects). Directory.Build.props/.targets are included once - after first include higher level ones will be skipped. This may cause some problems unless recursive include logic from dirs.props is applied to Directory.Build.* files as well.

I expect we will recursively include them if we nest them.

Yes, this is expected with Directory.Build.*. The first one gets imported, and then if there are parent props/targets files, they need to be explicitly included up the chain. This allows props/target authors to get the ordering right, which is important especially in props files.

We do this in the dotnet/machinelearning repo: https://github.com/dotnet/machinelearning/blob/master/src/Directory.Build.props#L3

weshaggard · 2018-05-21T22:37:53Z

src/Common/perf/PerfRunner/PerfRunner.csproj

-<?xml version="1.0" encoding="utf-8"?>
-<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+<Project Sdk="Microsoft.NET.Sdk">


@eerhardt are you aware of any way we can avoid setting this string in all our projects? We are very likely going to need to move to another SDK as part of our arcade repo and it would be great if we could just change it in one place in the repo if possible.

Based on dotnet/msbuild#1493, we can use <Import> semantics, because in Project Sdk="$(prop)", prop does not be evaluated, nor in <Sdk Name="$(prop)">. Alternatively, we can implement ISdkResolver when it is implemented in MSBuild. :)

You can do something like the following commit:

eerhardt/machinelearning@ed3c014

This allows my .csproj files to be built from the command line using dotnet build.

However, VS 2017 throws an error trying to load this project:

--------------------------- Microsoft Visual Studio --------------------------- Project file is incomplete. Expected imports are missing. --------------------------- OK ---------------------------

@davkean - Any thoughts on this VS error?

@weshaggard - honestly, I think attempting to put the SDK name in a single place is going to be an uphill battle, and I'm not sure it is worth the effort. Yes, it will change one, maybe two more times. But IMO the work and effort of find/replace in a few hundred files is less than trying to do things that the tools weren't designed to support. And note that you need 2 imports in every .csproj with my approach above (props on top, targets on bottom).

I agree it isn't a big deal. We should however be sure to keep the version out of all the project files as we we need to be able to override that in some cases like source-build so lets keep the version in the global.json file.

The error above indicates that you are not importing Microsoft.Common.targets and indirectly Microsoft.Managed.DesignTime.targets. You can verify this by doing a msbuild /pp on the project.

You can import the SDK in a central props/targets, but then the central props/targets can't be Directory.Build.*. Chicken and egg. Sdk imports common targets and common targets imports Directory.Build.*

If you use explicit dir.props/dir.targets, then you can do this: nguerrera/sdkimportrepro@6d34578

Oh I see that @eerhardt explicitly imported Directory.Build.* and then turned off the magic import. That should have worked too. My above commit with different name for explicit directory props/targets is working fine in VS. Let me try @eerhardt's approach in that sandbox.

nguerrera/sdkimportrepro@ab90e62 works fine in VS for me too. I cannot repro what @eerhardt is seeing with that approach.

I tried it on today's daily dogfood build, and am no longer seeing the error. The project loads and builds correctly in VS.

wtgodbe · 2018-05-21T22:38:06Z

I have not made any buildtools changes yet - the current state leaves me with the following error:

E:\code\corefx\Tools\dotnetcli\sdk\2.1.300-rc1-008673\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018: The "ResolvePackageAssets" task failed unexpectedly. [E:\code\corefx\src\System.Buffers\src\System.Buffers.csproj]
E:\code\corefx\Tools\dotnetcli\sdk\2.1.300-rc1-008673\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018: NuGet.Packaging.Core.PackagingException: Unable to find fallback package folder 'F:\git\corefx\Tools\dotnetcli\sdk\NuGetFallbackFolder'. [E:\code\corefx\src\System.Buffers\src\System.Buffers.csproj]

The fallback package folder location, F:\git\corefx\Tools\dotnetcli\sdk\NuGetFallbackFolder, is surprising to me. The base location (F:\git) was present in be0421d#diff-6483fde6c45c90fdfe98ca72eadfa651R7, and was a remnant of changes I brought in from Jose's original branch, but I've since corrected it in f60fa38#diff-6483fde6c45c90fdfe98ca72eadfa651R7, and I don't see any other places where F:\git is hardcoded.

weshaggard · 2018-05-21T22:38:28Z

Did you require any buildtools changes with this? do you mind sharing those if you did?

Yes I would expect a buildtools change as well.

weshaggard · 2018-05-21T22:39:36Z

src/Directory.Build.targets

+<Project InitialTargets="AddSkipGetTargetFrameworkToProjectReferences">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), dir.targets))\dir.targets" />
+  <Target Name="_CheckForInvalidConfigurationAndPlatform" />
+  <Target Name="_CheckForUnsupportedTargetFramework" />


Perhaps it is part of your buildtools changes but how are we blocking the import of the csharp targets?

I haven't explicitly done this yet, maybe a change I will have to implement in buildtools.

safern · 2018-05-21T22:48:31Z

The fallback package folder location, F:\git\corefx\Tools\dotnetcli\sdk\NuGetFallbackFolder, is surprising to me. The base location (F:\git) was present in be0421d#diff-6483fde6c45c90fdfe98ca72eadfa651R7, and was a remnant of changes I brought in from Jose's original branch, but I've since corrected it in f60fa38#diff-6483fde6c45c90fdfe98ca72eadfa651R7, and I don't see any other places where F:\git is hardcoded.

Maybe because the project.assets.json path?

f60fa38#diff-2a844428e665aad62dc165df96e249bdR18

wtgodbe · 2018-05-21T22:51:31Z

@safern yep, that's definitely it... not sure how I missed that in the repo search. @joperezr should I remove both the RestoreOutputPath and project.assets.json altogether?

joperezr · 2018-05-21T23:03:57Z

To be honest, I don't remember entirely what that particular RestoreOutputPath was doing there, so I don't have a good answer to you. What I do know, is that the first project I converted and tried to get to compile succesfully was System.Runtime, which is probably related to the property. I would just remove it for now and try to build and see why/if it was needed.

wtgodbe · 2018-05-21T23:22:05Z

Updated to remove the project.assets.json file, now hitting the following error while running dotnet.exe msbuild System.Buffers.csproj:

E:\code\corefx\Tools\dotnetcli\sdk\2.1.300-rc1-008673\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(197,5): error : Assets file 'E:\code\corefx\src\System.Buffers\src\project.assets.json' not found. Run a NuGet package restore to generate this file. [E:\code\corefx\src\System.Buffers\src\System.Buffers.csproj]

wtgodbe · 2018-05-22T23:05:03Z

Current state-

With the above error, it appears the error is happening because we're running the target ResolvePackageAssets against a project that hasn't had restore run on it. It seems correct to me not to run restore on most CoreFx projects, since they don't restore any packages. ResolvePackageAssets runs because ResolveLockFileReferences depends on it, which runs because ResolvePackageDependenciesForBuild depends on it, which runs as a BeforeTarget of AssignProjectConfiguration. This all happens in Microsoft.PackageDependencyResolution.targets, an SDK targets file.

The only way to conditionally disable any of these targets is by setting DesignTimeBuild = true, and not having a Project Asset File present (These are the conditions for ResolvePackageDependenciesForBuild). I'm not sure what the implications of setting DesignTimeBuild = true are yet, though in trying it I hit another failure involving a missing contract (no .dll for the desired project in bin/ref).

As for AssignProjectConfiguration, it appears that runs because ResolveReferences depends on it, and it is conditioned on '$(CurrentSolutionConfigurationContents)' != '' or '@(ProjectReference)'!=''. This happens in Microsoft.Common.CurrentVersion.targets. Trying to decide what the best way to proceed from here is.

davkean · 2018-05-22T23:08:32Z

@wtgodbe Do not set DesignTimeBuild to true. This is intended for VS purposes only.

You should call Restore on the project - Restore is more than just packages, it's also contains information on indirect P2Ps which are promoted to real P2Ps in SDK-based projects.

weshaggard · 2018-05-23T00:28:13Z

@wtgodbe I agree with @davkean that we shouldn't hack this and set DesignTimeBuild.

@davkean for our corefx projects we have been disabling restore to avoid the extra cost for each project. We don't plan to use PackageReference's and there will be limited ProjectReferences in our repo, as we plan to override the target framework to reference a folder of refs that we build in our repo. Do you think the performance is good enough to try and enable restore for all our projects even though we expect it to be a no-op for almost all? Or should we try a disable restore in our common targets?

davkean · 2018-05-23T00:34:15Z

Whether performance is good is the eye of the beholder; you should try using using implicit restore via MSBuild /restore switch or via dotnet build first. We've worked really hard to improve performance for this case. In VS, we're going to restore regardless - and if you want consistent behavior based on whether you only use command-line or used VS -> switched to command-line, I would just always restore.

weshaggard · 2018-05-23T00:50:10Z

Thanks @davkean we definitely want to try and have consistency between command line and VS. We will try the default restore way first and measure before making a decision to disable it or not.

In VS, we're going to restore regardless

That isn't controllable via an msbuild property from the project?

davkean · 2018-05-23T00:54:43Z

You can turn if via NuGet.config to prevent it from occurring automatically (say when you modify a project file or switch branches) and/or during build - but I see no reason why you would turn this on, unless it was causing issues, at which point we'd want bugs to fix said issues.

Using Visual Studio is much nicer if you just let us do our thing to make the project in the state we think it should be.

davkean · 2018-05-23T00:58:54Z

and if you want consistent behavior based on whether you only use command-line or used VS -> switched to command-line, I would just always restore..

On this, by consistent behavior, I mean you might get different build errors based on whether you had previously restored using VS or not. For example, let's say you have A -> B -> C. For normal SDK-based projects, 'A' can use 'C'. If you turn off restore in command-line but not for VS - your going to different build error based on whether you've ever opened in VS or not.

ghost · 2018-05-23T04:40:15Z

Probably we don't need to explicitly run dotnet restore or dotnet msbuild /restore

"cd into any corefx project directory and run dotnet build for default behavior (e.g. build) or dotnet build /p:MyProp=MyVal /t:MySpecialTarget"
is much cooler and popular than dotnet msbuild.

weshaggard · 2018-05-23T19:32:27Z

Probably we don't need to explicitly run dotnet restore or dotnet msbuild /restore

I agree for individual project builds we will just do dotnet build, but depending on the performance impact across the building of all projects we may do an upfront restore of all projects before we do a build, we will have to see the impact first.

wtgodbe · 2018-06-29T21:57:14Z

Never mind, I was building wrong. Build looks good in my (older) version of VS2017. I'm running a code coverage test run right now - it's taking a while, but seems to be working so far.

wtgodbe · 2018-06-29T22:22:17Z

I got the following test failures while running build-test -coverage on Windows:

E:\code\corefx\Tools\tests.targets(522,5): error : One or more tests failed while running tests from 'System.Console.Tests' please check E:\code\corefx\bin/tests/System.Console.Tests/netcoreapp-Windows_NT-Debug-x64/testResults.xml for details! [E:\code\corefx\src\System.Console\tests\System.Console.Tests.csproj]
E:\code\corefx\Tools\tests.targets(522,5): error : One or more tests failed while running tests from 'System.Globalization.CalendarsWithConfigSwitch.Tests' please check E:\code\corefx\bin/tests/System.Globalization.CalendarsWithConfigSwitch.Tests/netcoreapp-Windows_NT-Debug-x64/testResults.xml for details! [E:\code\corefx\src\System.Globalization.Calendars\tests\CalendarTestWithConfigSwitch\System.Globalization.CalendarsWithConfigSwitch.Tests.csproj]
E:\code\corefx\Tools\tests.targets(522,5): error : One or more tests failed while running tests from 'Invariant.Tests' please check E:\code\corefx\bin/tests/Invariant.Tests/netcoreapp-Windows_NT-Debug-x64/testResults.xml for details! [E:\code\corefx\src\System.Globalization\tests\Invariant\Invariant.Tests.csproj]
E:\code\corefx\Tools\tests.targets(522,5): error : One or more tests failed while running tests from 'System.IO.IsolatedStorage.Tests' please check E:\code\corefx\bin/tests/System.IO.IsolatedStorage.Tests/netcoreapp-Windows_NT-Debug-x64/testResults.xml for details! [E:\code\corefx\src\System.IO.IsolatedStorage\tests\System.IO.IsolatedStorage.Tests.csproj]

I did get a coverage report, though. I'll compare that to the result of just running build-test to see if the errors are specific to the coverage run.

wtgodbe · 2018-06-29T22:49:44Z

This is what I get running a normal test build:

E:\code\corefx\dir.traversal.targets(77,5): error : (No message specified) [E:\code\corefx\src\tests.builds]

Build FAILED.

E:\code\corefx\Tools\tests.targets(514,5): warning : System.Console.Tests Total: 120, Errors: 0, Failed: 11, Skipped: 2, Time: 50.245s [E:\code\corefx\src\System.Console\tests\System.Console.Tests.csproj]
E:\code\corefx\Tools\tests.targets(514,5): warning MSB3073: The command "E:\code\corefx\bin/tests/System.Console.Tests/netcoreapp-Windows_NT-Debug-x64//RunTests.cmd E:\code\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/" exited with code 1. [E:\code\corefx\src\System.Console\tests\System.Console.Tests.csproj]
E:\code\corefx\Tools\tests.targets(522,5): error : One or more tests failed while running tests from 'System.Console.Tests' please check E:\code\corefx\bin/tests/System.Console.Tests/netcoreapp-Windows_NT-Debug-x64/testResults.xml for details! [E:\code\corefx\src\System.Console\tests\System.Console.Tests.csproj]
E:\code\corefx\dir.traversal.targets(77,5): error : (No message specified) [E:\code\corefx\src\tests.builds]

eerhardt · 2018-07-06T00:30:16Z

I'm removing the WIP on this PR. I believe this is ready for review and to be merged.

All the legs should be passing - I will fix them if they aren't.
I've tested out the VS experience. Overall, things work very well. However, there are a few bugs I've run into:

VS consumes a lot of CPU in the background High CPU when target framework is empty project-system#3670 (comment)
The editor doesn't pick up the new properties after switching configurations Editor doesn't handle conditional compilation symbol with netstandard or .net core project-system#2733 (comment) (unloading and reloading the project fixes it)
F5 a test project no longer launches it for debugging. Setting RunCommand/StartProgram on ClassLibrary doesn't work in new project system project-system#3684

@weshaggard @wtgodbe - please take a look. Since this change is so big, it will probably be easiest to review this change commit-by-commit, and possibly skipping the merge commits. Unless you want to pull the change locally and review it that way.

weshaggard · 2018-07-06T00:51:51Z

Directory.Build.props

+  <!-- Set up default paths for reference assemblies -->
+  <PropertyGroup Condition="'$(IsReferenceAssembly)'=='true'">
+    <!-- Create a common root output directory for all reference assemblies -->
+    <ReferenceAssemblyOutputPath Condition="'$(ReferenceAssemblyOutputPath)' == ''">$(BaseOutputPath)ref/</ReferenceAssemblyOutputPath>


Why did these get moved here instead of using the ones in BuildTools? https://github.com/dotnet/buildtools/blob/16a6d1f39cf0a572041d51bd318913548ba41b34/src/Microsoft.DotNet.Build.Tasks/PackageFiles/ReferenceAssemblies.targets#L15

I’m not entirely sure why this changed. I asked @wtgodbe and he said that change was included with @joperezr’s initial changes. I can try reverting it to see what happens.

I'd like to understand if it is needed here still.

I've investigated why this was being set.

The reason looks to be that the BuildTools setting of $(OutputPath) is now occurring too late in the project evaluation. Before SDK-style projects, BuildTools' ReferenceAssemblies.targets is imported before Microsoft.Common.targets. Now, with SDK-style projects, ReferenceAssemblies.targets is being imported after Microsoft.Common.targets.

This change was attempting to set these properties earlier, but aren't the real fix since $(AssemblyVersion) is not set yet. To fix this, I opened a BuildTools PR: dotnet/buildtools#2091. After we take that fix into corefx, we can remove this section and use the ones in BuildTools.

weshaggard · 2018-07-06T00:56:03Z

src/System.Net.NetworkInformation/src/System.Net.NetworkInformation.csproj

@@ -5,7 +5,7 @@
    <ProjectGuid>{3CA89D6C-F8D1-4813-9775-F8D249686E31}</ProjectGuid>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <ILLinkClearInitLocals>true</ILLinkClearInitLocals>
-    <Configurations>netcoreapp-Linux-Debug;netcoreapp-Linux-Release;netcoreapp-OSX-Debug;netcoreapp-OSX-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release</Configurations>
+    <Configurations>netcoreapp-Linux-Debug;netcoreapp-Linux-Release;netcoreapp-OSX-Debug;netcoreapp-OSX-Release;netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release</Configurations>


Do you know if these have to be static and directly in the project? I'm asking mainly to try and determine if we can combine the Debug/Release aspects with our existing BuildConfigurations property to eliminate the duplication.

No I don’t know for sure. @davkean or @Pilchie - do you know off the top of your head?

I believe it has to be known at evaluation time (i.e. Not the result of a target), but doesn't have to be directly in the project. @davkean would know for sure.

weshaggard

Looks good overall. Thanks for pushing this through.

We probably need to go and update some documentation around how to build and edit projects. Can you please take a pass through and make some updates?

eerhardt · 2018-07-06T23:11:57Z

@dotnet-bot test this please

eerhardt · 2018-07-08T14:15:51Z

@dotnet-bot test this please

eerhardt · 2018-07-08T17:23:21Z

Test Windows x64 Debug Build

eerhardt · 2018-07-08T19:06:41Z

Test Windows x64 Debug Build

Convert all projects to SDK-style Commit migrated from dotnet/corefx@0b0f63d

wtgodbe added 3 commits May 21, 2018 11:53

Convert all .csproj files in src to SDK-style projects

91d7e62

Add Directory.build.props/targets

be0421d

Add project.assets.json & edit properties

f60fa38

wtgodbe added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label May 21, 2018

wtgodbe changed the title ~~Convert all projects to SDK-style~~ [WIP] Convert all projects to SDK-style May 21, 2018

weshaggard reviewed May 21, 2018

View reviewed changes

Remove project.assets.json file

10b7151

wtgodbe force-pushed the SdkProj branch from e1ec699 to 10b7151 Compare May 22, 2018 19:40

karelz added the area-Infrastructure-libraries label May 22, 2018

karelz assigned wtgodbe May 22, 2018

eerhardt added 3 commits July 5, 2018 14:06

Merge master into SdkProj

0343b37

Run UpdateVSConfigurations on the current code.

1a4b8a2

Fix up new projects and usages of dir.props/targets.

9b7b8c0

eerhardt force-pushed the SdkProj branch from 9aa71d1 to 9b7b8c0 Compare July 5, 2018 23:36

This was referenced Jul 5, 2018

High CPU when target framework is empty dotnet/project-system#3670

Closed

Editor doesn't handle conditional compilation symbol with netstandard or .net core dotnet/project-system#2733

Closed

Merge master into SdkProj

e72643b

eerhardt changed the title ~~[WIP] Convert all projects to SDK-style~~ Convert all projects to SDK-style Jul 6, 2018

weshaggard reviewed Jul 6, 2018

View reviewed changes

eerhardt removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 6, 2018

weshaggard reviewed Jul 6, 2018

View reviewed changes

weshaggard approved these changes Jul 6, 2018

View reviewed changes

eerhardt added 2 commits July 6, 2018 14:51

Set TargetFramework to workaround project system bug.

aa89672

Merge remote-tracking branch 'upstream/master' into SdkProj

b86cade

eerhardt mentioned this pull request Jul 6, 2018

Need to re-default output properties after writing $(OutputPath). dotnet/buildtools#2091

Merged

Merge master into SdkProj

99e21e6

eerhardt merged commit 0b0f63d into dotnet:master Jul 8, 2018

eerhardt mentioned this pull request Jul 10, 2018

Remove ref assembly output workaround. #30950

Merged

karelz added this to the 3.0 milestone Jul 19, 2018

jkotas mentioned this pull request Aug 9, 2018

How to run ILVerify? dotnet/corert#6198

Closed

ViktorHofer mentioned this pull request Aug 14, 2018

Convert projects to SDK-style dotnet/wcf#3116

Closed

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Merge pull request dotnet/corefx#29831 from wtgodbe/SdkProj

300f352

Convert all projects to SDK-style Commit migrated from dotnet/corefx@0b0f63d

Convert all projects to SDK-style #29831

Convert all projects to SDK-style #29831

Conversation

wtgodbe commented May 21, 2018

joperezr commented May 21, 2018

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

eerhardt May 23, 2018 • 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

wtgodbe commented May 21, 2018 • edited Loading

weshaggard commented May 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

safern commented May 21, 2018

wtgodbe commented May 21, 2018

joperezr commented May 21, 2018

wtgodbe commented May 21, 2018 • edited Loading

wtgodbe commented May 22, 2018 • edited Loading

davkean commented May 22, 2018 • edited Loading

weshaggard commented May 23, 2018

davkean commented May 23, 2018

weshaggard commented May 23, 2018

davkean commented May 23, 2018

davkean commented May 23, 2018 • edited Loading

ghost commented May 23, 2018

weshaggard commented May 23, 2018

wtgodbe commented Jun 29, 2018

wtgodbe commented Jun 29, 2018 • edited Loading

wtgodbe commented Jun 29, 2018 • edited Loading

eerhardt commented Jul 6, 2018 • 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

weshaggard left a comment

Choose a reason for hiding this comment

eerhardt commented Jul 6, 2018

eerhardt commented Jul 8, 2018

eerhardt commented Jul 8, 2018

eerhardt commented Jul 8, 2018

eerhardt May 23, 2018 •

edited

Loading

wtgodbe commented May 21, 2018 •

edited

Loading

wtgodbe commented May 21, 2018 •

edited

Loading

wtgodbe commented May 22, 2018 •

edited

Loading

davkean commented May 22, 2018 •

edited

Loading

davkean commented May 23, 2018 •

edited

Loading

wtgodbe commented Jun 29, 2018 •

edited

Loading

wtgodbe commented Jun 29, 2018 •

edited

Loading

eerhardt commented Jul 6, 2018 •

edited

Loading