Generate MSBuild property that has the path to a package #2271

jeffkl · 2018-05-31T18:28:08Z

Some packages have a weird layout. They are in use via packages.config because you can guess the path with something like ..\packages\Some.Bad.Package.1.0.0\something.exe. When migrating to PackageReference, these packages are very hard to consume. Not all of these packages can be updated so there's a need to add a feature to NuGet so you can use them.

This change adds a GeneratePathProperty metadata item to <PackageReference /> and then generates a property in the .g.props.

Example:

<Project>
  <ItemGroup>
      <PackageReference Include="Some.Bad.Package" Version="1.0.0" GeneratePathProperty="true" />
  </ItemGroup>

  <Target Name="TakeAction" AfterTargets="Build">
    <Exec Command="$(PkgSome_Bad_Package)\something.exe" />
  </Target>
</Project>

The resulting nuget.g.props contains:

<PropertyGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <PkgSome_Bad_Package Condition=" '$(PkgSome_Bad_Package)' == '' ">C:\Users\Bill\.nuget\packages\Some.Bad.Package\1.0.0</PkgSome_Bad_Package>
</PropertyGroup>

This change also generates a path property for any package that has a tools folder.

Example:

<Project>
  <ItemGroup>
      <PackageReference Include="ILMerge" Version="2.14.1208" />
  </ItemGroup>

  <Target Name="TakeAction" AfterTargets="Build">
    <Exec Command="$(PkgIlMerge)\tools\ilmerge.exe" />
  </Target>
</Project>

jeffkl · 2018-05-31T18:28:27Z

FYI @AndyGerlicher

emgarten · 2018-06-06T21:58:45Z

How about writing out the path to every package under PACKAGE_PATH_{package id} instead of requiring an attribute on packagereference?

As long as this is conditioned on TFM there can only be a single version of each id, so there wouldn't be conflicts.

I'm glad to see work being done in this area, I have long wanted this feature, and had planned at one time to write an external package that would write this out after restore.

A few issues I see with the PropertyName attribute:

By requiring this property it makes it difficult for transitive references. If a child project references the tool and parent projects also want to use it, all of them need to reference the tool directly, which makes it hard to change the version.
The name is generic and not intuitive, I would not expect the value here to later become an output property from restore if I just saw this in a csproj file.
Conflicts need to be handled in a deterministic way, for example if multiple references use the same property name.

jeffkl · 2018-06-06T22:18:38Z

I was initially writing out a property for every package but observed two issues:

SDK-style projects reference a lot of packages out of box and so you end up with a lot of extra properties that you don't need.
NuGet would have to generate the property using some naming algorithm. MSBuild properties cannot contain periods, so they would have to be something like packageId.Replace(".", "_"). Now if there are two packages like Foo.bar and Foo_bar, the properties will collide.

Forcing the user to opt into this behavior feels like the right solution to convey that its not supposed to be necessary. We want all packages to conform so you don't need to know where they are.

Also, by requiring the user to specify the property name, NuGet doesn't have to worry about the complicated algorithm to generating the property name.

By requiring this property it makes it difficult for transitive references. If a child project references the tool and parent projects also want to use it, all of them need to reference the tool directly, which makes it hard to change the version.

I really don't forsee the need to generate properties for them. If needed, a user could add an explicit package reference to get a property.

The name is generic and not intuitive, I would not expect the value here to later become an output property from restore if I just saw this in a csproj file.

I chose PropertyName because its what you use on an MSBuild task to capture output values. My thought was that this feature would be used exclusively by MSBuild so it should try to be like the Output element.

Conflicts need to be handled in a deterministic way, for example if multiple references use the same property name.

At the moment, its last-one-wins just like everything else in MSBuild. We could throw an exception instead.

Lastly, to get a property for all package references, a user can simply do this:

<ItemDefinitionGroup>
  <PackageReference>
    <!-- Generate a property like PkgFooBar for all PackageReferences -->
    <PropertyName>Pkg%(ItemSpec.Replace(".", ""))</PropertyName>
  </PackageReference>
</ItemDefinitionGroup>

But we wouldn't really want to encourage people to use this feature, we want to make it available for migration purposes to further the adoption of PackageReference.

emgarten · 2018-06-06T22:32:48Z

How about writing all package paths, but only if the user sets a flag to opt in then? You could also set a default prefix for the property names, or opt to not have a prefix at all.

I would say that not having paths for transitive packages will be much more painful than collisions on . -> _, I can't think of any packages off hand which would hit that.

We have seen a lot of complaints in the past about the need for adding top level references for things like this. When adding the feature and testing it on small solutions it seems fine, but with real world solutions it falls apart and becomes painful fast. We need to avoid these problems.

Does Pkg%(ItemSpec.Replace(".", "")) work for transitive references, or just top level packagereferences?

I don't see this as a migration feature. There are many scenarios where users need to override how NuGet references dlls from a package. For example with externing a namespace, if for example you needed to do that on a transitive reference you would want to find the current references and modify them through MSBuild. If you had all package paths automatically I think it would be much easier.

jeffkl · 2018-06-07T14:53:09Z

How about writing all package paths, but only if the user sets a flag to opt in then?

Can you describe more what you mean? Do you mean a property for every single package using an algorithm that comes up with the property name? Or just writing to a file every path? I want to understand your idea more.

I would say that not having paths for transitive packages will be much more painful than collisions on . -> _,

Do you not think its enough to just have an explicit reference? Or to say that packages that are malformed can't be used transitively in non-traditional ways?

Does Pkg%(ItemSpec.Replace(".", "")) work for transitive references, or just top level packagereferences?

No, but my position is still that we don't need to provide properties for indirect dependencies since I am of the opinion that people can just have an explicit dependency on anything that they need that stock NuGet doesn't provide.

The purpose of this feature in my view is a workaround to consume non-conformant packages. I believe if we wanted more functionality with PackageReference we should just add them as first-class features. Externing namespaces for example should be something that's just built in at some point. And using property path hints is the temporary workaround.

When adding the feature and testing it on small solutions it seems fine, but with real world solutions it falls apart and becomes painful fast. We need to avoid these problems.

In my experience of onboarding small, medium, and large repos, only a very small percentage of the packages can't be used out-of-the-box with PackageReference. When a team needs to know the path, there's usually 1 to 5 packages that a repository is using that no one owns and can't be updated. 90% of the time its a command-line tool that they are executing during their build and the package doesn't have a build\packageid.props that sets a property to the path of the EXE. I haven't come across a single repository that needs to know the path to every single package.

jainaashish · 2018-06-07T17:35:45Z

Before even going into specifics of design, I'd recommend to first discuss the requirement and understand what do we have today? @rrelyea already mentioned on the original issue about already having something on the similar lines.

Even I'm not really able to understand how it's more painful to consume some tool from a package with PackageReference compare to packages.config? With packages.config, you'd guess the path something like ..\packages\MyTool.1.0.0\tool\mytool.exe and with PackageReference, it'd something like $(NuGetPackageRoot)\MyTool\1.0.0\tool\mytool.exe. Both cases consumer has to be aware about the package version and update it manually in case he updates the package.

And lastly, even I agree with Justin's point to covering the transitive dependencies scenario as well because IMO that's the real difference between packages.config vs PackageReference.

emgarten · 2018-06-07T18:33:05Z

Here is how I see the problem:

As a NuGet user I want to access packages that are part of my project through MSBuild to customize and extend my build process.
Restore is a black box. I should not need to know the version that restore picked, or any environment settings such as the packages folder path(s).
All packages in the graph should be treated the same, this problem isn't specific to any one type of package, package asset, direct, or transitive reference.

For an actual tool package I would expect the download API to be used since these package are not used by a project. The best experience for tools so far is nuget.exe install -ExcludeVersion, it makes it trivial to reference assets from the package however you want.

@jainaashish

rrelyea already mentioned on the original issue about already having something on the similar lines.

I don't think anything like this exists already. Restore will write out the package folders, from there you could find the package.

The problem is similar to packages.config, but it is a bit more tricky since the packages folder can change with the environment, and it is hard to take advantage of new features with PR such as floating versions.

In the NuGet.Client build we workaround the problem by downloading many packages to a known location and then referencing them directly. If this problem were fixed you could get rid of this file completely: https://github.com/NuGet/NuGet.Client/blob/dev/.nuget/packages.config and the build wouldn't have to download nuget.exe at all to bootstrap 😄

@jeffkl
Finding the path to a package is fundamental scenario. I agree that it is most common for tools, but it is common for many other scenarios.

Changing LibraryDependency here is equivalent to changing ItemSpec in MSBuild. NuGet can't 1st class everything for the same reasons that MSBuild can't. NuGet is used for C++ and 3rd party project systems which aren't part of VS. Users need a generic way to add custom functionality around packages.

Do you mean a property for every single package using an algorithm that comes up with the property name?

<WritePackagePathProperties>true</WritePackagePathProperties> would give you:

<PACKAGE_PATH_NEWTONSOFT_JSON>c:\packages\newtonsoft.json\9.0.1\</PACKAGE_PATH_NEWTONSOFT_JSON>
<PACKAGE_PATH_SYSTEM_RUNTIME>c:\packages\system.runtime\4.3.0\</PACKAGE_PATH_SYSTEM_RUNTIME>

jeffkl · 2018-06-12T14:53:39Z

@rrelyea there is no current feature to do what we are requesting. There seems to be two options:

Create properties only for packages that a user specifies using the exact property name provided.
When a user specifies, create a property for all packages using an algorithm that comes up with the property name.

Which of these should we go after?

jeffkl · 2018-06-12T22:13:40Z

Samples of generating a property for every package: https://gist.github.com/jeffkl/6acdda0b4116b98244700685927a7cac

jainaashish · 2018-06-12T22:23:19Z

Thanks @emgarten for the explanation. I agree that as a NuGet user, I should always be able to access my packages and consume them in build or publish workflow. This issue existed with packages.config as well but has become more prominent with PackageReference. So we'll discuss it and come up with the right plan to address that.

jeffkl · 2018-06-19T20:44:25Z

@jainaashish and @nkolev92 please review, I've updated it according to our discussion.

AndyGerlicher · 2018-06-25T17:42:10Z

Ping please @jainaashish and @nkolev92

nkolev92

Seems like the right approach, but I'd rather see it follow the same pattern as the rest of the props generation code.

Please add tests in https://github.com/NuGet/NuGet.Client/blob/dev/test/NuGet.Core.Tests/NuGet.Commands.Test/BuildAssetsUtilsTests.cs

nkolev92 · 2018-06-25T17:42:27Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

-                        .Select(e => pkg.Value.GetAbsolutePath(e)))
-                        .Select(path => GetPathWithMacros(path, repositoryRoot))
-                        .Select(GenerateImport));
+                        pkg.Key.Build.WithExtension(TargetsExtension)


nit: the indentation of this still looks off. :)

I did not mean to touch the whitespace so I'll revert all that

nkolev92 · 2018-06-25T17:42:46Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

-                        .Select(e => pkg.Value.GetAbsolutePath(e)))
-                        .Select(path => GetPathWithMacros(path, repositoryRoot))
-                        .Select(GenerateImport));
+                        pkg.Key.Build.WithExtension(PropsExtension)


nit: same as above.

nkolev92 · 2018-06-25T17:53:51Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

+                // Create an empty PropertyGroup for package properties
+                var packagePathsPropertyGroup = MSBuildRestoreItemGroup.Create("PropertyGroup", Enumerable.Empty<XElement>(), 1000, isMultiTargeting ? frameworkConditions : Enumerable.Empty<string>());
+
+                var projectGraph = targetGraph.Graphs.FirstOrDefault();


Why not use the flattened graph?
targetGraph.Flattened?

Under the debugger, I found that the first graph was the project itself which has its direct dependencies. Would targetGraph.Flattened give all transitive dependencies as well? Technically the GeneratePathProperty wouldn't be set on any transitive dependencies.

Under the debugger, I found that the first graph was the project itself which has its direct dependencies.

I'm not sure if that's the right statement, I'd double check it. Or filter it based on GraphItem

nkolev92 · 2018-06-25T18:05:04Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs


                props.AddRange(GenerateGroupsWithConditions(buildPropsGroup, isMultiTargeting, frameworkConditions));

+                // Create an empty PropertyGroup for package properties
+                var packagePathsPropertyGroup = MSBuildRestoreItemGroup.Create("PropertyGroup", Enumerable.Empty<XElement>(), 1000, isMultiTargeting ? frameworkConditions : Enumerable.Empty<string>());


Anything preventing you from using the same pattern as the build props code?

This one is a little different in that its a property group with conditions vs an import group with no conditions containing imports with conditions. But if you feel strongly I can try to follow a pattern

Not sure if I understand correctly but we already add PropertyGroup with condition in nuget.g.props file. wouldn't this be similar to that? We can have a dict of packages path and add them in one shot.

jeffkl · 2018-06-26T20:36:10Z

@nkolev92 let me know what you think of the unit test I wrote and I'll add a few more. My unit test tests the whole thing from end to end.

jeffkl · 2018-06-28T16:14:19Z

@nkolev92 does this look ready? Can you initiate the CI tests please?

nkolev92 · 2018-06-28T23:05:02Z

@jeffkl
There were some lovely issues with the build, can you please rebase to make all tests run?

jainaashish · 2018-06-28T23:15:10Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

+                var packagePathProperties = packagesWithTools.Union(projectGraph.Item.Data.Dependencies.Where(i => i.GeneratePathProperty).Select(i => i.Name)).Distinct(StringComparer.OrdinalIgnoreCase)
+                    .Select(i => sortedPackages.Cast<KeyValuePair<LockFileTargetLibrary, Lazy<LocalPackageSourceInfo>>?>().FirstOrDefault(x => x.Value.Key.Name.Equals(i, StringComparison.OrdinalIgnoreCase)))
+                    .Where(i => i != null && i.Value.Value.Exists())
+                    .Select(i => GeneratePackagePathProperty(i.Value.Key.Name, i.Value.Value.GetAbsolutePath(i.Value.Value.Value.Package.ExpandedPath)));


Can you please simply this logic? these complex linq queries are never easy to read and understand. besides, i also dont understand this i.Value.Value.Value.Package.ExpandedPath

I agree, the Linq statements in this file really tested my understanding. I'll break them up a little.

jainaashish · 2018-06-28T23:20:28Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

@@ -493,6 +495,24 @@ public static string GetMSBuildFilePath(PackageSpec project, RestoreRequest requ

                props.AddRange(GenerateGroupsWithConditions(buildPropsGroup, isMultiTargeting, frameworkConditions));

+                // Create an empty PropertyGroup for package properties
+                var packagePathsPropertyGroup = MSBuildRestoreItemGroup.Create("PropertyGroup", Enumerable.Empty<XElement>(), 1000, isMultiTargeting ? frameworkConditions : Enumerable.Empty<string>());


Where are you adding PropertyGroup condition for ExcludeRestorePackageImports?

Those are added on line 573.

jainaashish · 2018-06-28T23:22:05Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

+
+        private static XElement GeneratePackagePathProperty(string packageName, string packagePath)
+        {
+            return GenerateProperty($"Pkg{packageName.Replace(".", "_")}", packagePath);


Where are we adding condition for Pkg<package_path> already exists or not on this property?

They are added by GenerateProperty on line 191

jainaashish · 2018-06-28T23:27:10Z

test/NuGet.Core.Tests/NuGet.Commands.Test/BuildAssetsUtilsTests.cs

@@ -611,5 +619,108 @@ public void BuildAssetsUtils_VerifyPositionAndSortOrder()
                Assert.Equal("x", targetItemGroups[3].Attribute(XName.Get("Condition")).Value.Trim());
            }
        }
+
+        [Fact]
+        public async Task BuildAssetsUtils_GeneratePathProperty()


can we also add test for a package with tool folder and verify that it does generate path property implicitly?

jainaashish

Overall looks good, i just have few concerns regarding readability and missing conditions on PropertyGroup.

jeffkl · 2018-07-02T15:14:52Z

@nkolev92 I've rebased, please relaunch the CI build

jainaashish · 2018-11-06T18:18:45Z

ahh it wasn't implicit, we have to update https://github.com/jainaashish/project-system/blob/a8cf44c3948757b7cb261532834ce546bb5fb47a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/PackageReference.xaml in order to get this metadata as part of nomination.

I can raise the PR to fix it, but it's upto SDK Project System team about the timeline. @Pilchie

KirillOsenkov · 2019-01-12T01:20:59Z

@radical do you know if or when this would be available in Mono? I just tried GeneratePathProperty on Mono 5.18.0.234 and it didn't appear to have generated the Pkg* property.

nkolev92 · 2019-01-12T01:24:23Z

@KirillOsenkov

Depends on the nuget.exe version the mono isntallation is carrying.

This change is in 4.9, which I'm pretty sure mono 5.8.0 does not have.

radical · 2019-01-12T01:30:31Z

I'm guessing that @KirillOsenkov meant 5.18.x . We recently bumped to nuget 4.8.1 (mono/mono#12109). Could you open an issue under mono for this?

KirillOsenkov · 2019-01-12T01:39:35Z

@radical yes, 5.18, thanks. Filed mono/mono#12393.

ericstj · 2019-03-01T17:42:06Z

src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs

+
+                // Find the packages with matching IDs in the list of sorted packages, filtering out ones that there was no match for or that don't exist
+                var packagePathProperties = localPackages
+                    .Where(pkg => pkg?.Value?.Package != null && packageIdsToCreatePropertiesFor.Contains(pkg.Value.Package.Id) && pkg.Exists())


I hit a case-sensitivity issue here. I believe this contains call is the cause since I don't see how it handles if the winner of the distinct call above had different case than the value in Package.Id.

NuGet/Home#7843

I think I've hit this too but wasn't able to figure out the problem. I think you're right that its casing. I'll submit a PR

bording · 2019-03-01T18:30:11Z

I've been using VS 2019 RC with SDK-style projects, and I've noticed that some packages seem to be getting path entries in nuget.g.props without me specifying GeneratePathProperty="true" on the package reference. Is this expected? I thought it was suppose to be opt-in?

If it is supposed to be happening by default, then there's still a problem because I'm not seeing all of the packages in a given project have the entries, just some of them.

dasMulli · 2019-03-01T18:50:19Z

I have observed the same behaviour, did the default change?
Not against it btw, I think is is quite useful.

jeffkl · 2019-03-01T19:18:25Z

The default was from the beginning to generate a property for any package that contains a tool folder assuming that it didn't have any reference assemblies or build logic and the only way to use the tool would be to know the path to the tool. However there was a feature missed in Visual Studio 2017 where the properties were generated for SDK-style projects. I think the bug fix is in 2019 so you're seeing the properties generated for the first time as they are expected.

bording · 2019-03-01T19:24:04Z

The default was from the beginning to generate a property for any package that contains a tool folder

Well that does explain the behavior I'm seeing, but I have to say that it being conditional like that is super confusing. I'd much rather just have it be there always or never.

jeffkl · 2019-03-01T19:31:59Z

I think the argument could be made for either automatic or never/always. Maybe the best course of action would be to add a property you can set to disable the automatic creation of property for packages with tools. The default would still have to be automatic though...

bording · 2019-03-01T19:37:05Z

The default would still have to be automatic though.

That's fine, make it automatic for all package references then, not just ones with a certain internal structure.

I have scenarios where having the package path automatically created is going to be useful, and none of them involve the tools folder. Right now that means I need to always add GeneratePathProperty="true" to my package reference anyway, since I can't count on it being there, but that means it might be redundant and ends up as noise in the project file. Having to go inspect the generated restore props file to see if I might not need it is a horrible experience.

jeffkl · 2019-03-01T20:37:04Z

@bording I think you should open an issue and start a discussion. I probably don't fully understand your scenarios so having more info would help a lot. And that would also allow others to comment and give us an idea of whether or not to change the feature.

AndyGerlicher · 2019-03-04T17:51:42Z

@bording I realize this isn't a complete solution but it might help for your situation for now. You can add a Directory.Build.props file at the root of your repo containing:

<Project>
    <ItemDefinitionGroup>
        <PackageReference GeneratePathProperty="true" />
    </ItemDefinitionGroup>
</Project>

And all your package references will get that automatically without having to specify it each time.

bording · 2019-03-04T18:02:06Z

@AndyGerlicher Thanks for the suggestion. I would probably end up doing something like that for my repos.

However, as you say, that doesn't address the general concerns about the usability/discoverability of the feature, so I will raise a new issue as @jeffkl suggested.

nkolev92 · 2019-03-04T18:34:35Z

I remember we did discuss the all or nothing approach (I actually preferred it over the extra property), but there were some concerns about adding lots of potentially unnecessary evaluations from msbuild side.

Let's continue the discussion on the new issue but I just want to bring up that question again.

bording · 2019-03-04T22:53:56Z

Here's the new issue: NuGet/Home#7851

jzabroski · 2021-07-21T19:45:00Z

Samples of generating a property for every package: https://gist.github.com/jeffkl/6acdda0b4116b98244700685927a7cac

@jeffkl was this sample moved somewhere else. thanks

jeffkl force-pushed the createproperties branch from 0cad30a to 863e392 Compare June 19, 2018 20:36

nkolev92 reviewed Jun 25, 2018

View reviewed changes

jeffkl force-pushed the createproperties branch from 863e392 to 754da4c Compare June 26, 2018 17:38

jeffkl changed the title ~~WIP: Generate MSBuild property that has the path to a package~~ Generate MSBuild property that has the path to a package Jun 28, 2018

jainaashish added Approved for CI and removed Approved for CI labels Jun 28, 2018

jainaashish reviewed Jun 28, 2018

View reviewed changes

jainaashish suggested changes Jun 28, 2018

View reviewed changes

jeffkl force-pushed the createproperties branch from a45bd16 to 89d44c3 Compare July 2, 2018 14:50

jeffkl force-pushed the createproperties branch from f9a635e to 4731094 Compare July 2, 2018 15:22

jainaashish mentioned this pull request Nov 6, 2018

Passing GeneratePathProperty metadata as part of nomination dotnet/project-system#4226

Merged

KirillOsenkov mentioned this pull request Jan 12, 2019

NuGet: bump to latest NuGet to support GeneratePathProperty mono/mono#12393

Closed

nguerrera mentioned this pull request Feb 6, 2019

Upgrading Roslyn to 3.0.0-beta2-final dotnet/corefx#35134

Merged

tmat mentioned this pull request Feb 13, 2019

If we are using .NET Core 1.1 or 1.0 use the 1.0 xunit runner dotnet/arcade#2035

Merged

ericstj reviewed Mar 1, 2019

View reviewed changes

bording mentioned this pull request Mar 4, 2019

Paths to the package folder(GeneratePathProperty feature) should be generated for all libraries in the project graph NuGet/Home#7851

Open

tmat mentioned this pull request Mar 5, 2019

Add GenerateBuildManifest SdkTask dotnet/arcade#2155

Merged

nsmithdev mentioned this pull request Mar 19, 2019

PackageReference is missing copy files to output feature NuGet/Home#4837

Open

jaredpar mentioned this pull request Aug 9, 2019

Replace NugetPackageRoot with explicit package paths dotnet/roslyn#37882

Open

jeffkl mentioned this pull request Sep 30, 2019

Get package version from central store in projects microsoft/MSBuildSdks#132

Closed

christophe-zurn-sonarsource mentioned this pull request Jan 14, 2020

Remove use of NUGET_PACKAGES environment variable SonarSource/sonar-dotnet#3032

Closed

jzabroski mentioned this pull request Jul 21, 2021

Please provide a way to reference package ID and version in .target/.props files NuGet/Home#6431

Closed

jzabroski mentioned this pull request Jul 21, 2021

Add a parallel PkgVersion_PACKAGE_NAME variable for when PackageReference Version="*" microsoft/MSBuildSdks#272

Closed

Generate MSBuild property that has the path to a package #2271

Generate MSBuild property that has the path to a package #2271

Conversation

jeffkl commented May 31, 2018 • edited by nkolev92 Loading

jeffkl commented May 31, 2018

emgarten commented Jun 6, 2018

jeffkl commented Jun 6, 2018

emgarten commented Jun 6, 2018

jeffkl commented Jun 7, 2018

jainaashish commented Jun 7, 2018 • edited Loading

emgarten commented Jun 7, 2018

jeffkl commented Jun 12, 2018

jeffkl commented Jun 12, 2018

jainaashish commented Jun 12, 2018

jeffkl commented Jun 19, 2018

AndyGerlicher commented Jun 25, 2018

nkolev92 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

jeffkl commented Jun 26, 2018

jeffkl commented Jun 28, 2018

nkolev92 commented Jun 28, 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

jainaashish left a comment

Choose a reason for hiding this comment

jeffkl commented Jul 2, 2018

jainaashish commented Nov 6, 2018

KirillOsenkov commented Jan 12, 2019 • edited Loading

nkolev92 commented Jan 12, 2019

radical commented Jan 12, 2019

KirillOsenkov commented Jan 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bording commented Mar 1, 2019

dasMulli commented Mar 1, 2019

jeffkl commented Mar 1, 2019

bording commented Mar 1, 2019

jeffkl commented Mar 1, 2019

bording commented Mar 1, 2019 • edited Loading

jeffkl commented Mar 1, 2019

AndyGerlicher commented Mar 4, 2019

bording commented Mar 4, 2019

nkolev92 commented Mar 4, 2019

bording commented Mar 4, 2019

jzabroski commented Jul 21, 2021

jeffkl commented May 31, 2018 •

edited by nkolev92

Loading

jainaashish commented Jun 7, 2018 •

edited

Loading

KirillOsenkov commented Jan 12, 2019 •

edited

Loading

bording commented Mar 1, 2019 •

edited

Loading