ProjectReferences Negotiate SetPlatform Metadata #6655

benvillalobos · 2021-07-06T18:17:23Z

Context

TargetFrameworks are determined dynamically at build time, this change sets a baseline for performing SetPlatform negotiation.

In a project setup like so:

A --> B --> C

A can build as x86
B as x86, x64
C as AnyCPU

Projects can tell ProjectReferences to build as their "most compatible" platform.

A would tell B to build as x86, and B would tell C to build AnyCPU.

The build can now negotiate what Platform a ProjectReference should build as. This negotiation happens in the _GetProjectReferencePlatformProperties target. It piggybacks off of the GetTargetFrameworks call that happens in _GetProjectReferenceTargetFrameworkProperties that now pulls $(Platforms) data from referenced projects. The GetCompatiblePlatform task performs the main negotiation. An optional PlatformLookupTable can be specified per ProjectReference, or in individual projects, or in a Directory.Build.props. This lookup table is in the form foo=bar;C=D and means (in English), "When some project is building with platform foo, and a ProjectReference can build with platform B, tell the referenced project to build as bar"

These are the rules by which the GetCompatiblePlatform task assigns a platform:

Can the referenced project build with the same platform as the currently-building project? Use that.
Does the ProjectReference item have a PlatformTranslationTable defined? Use that to find a mapping if possible
Does the currently-building project have a PlatformTranslationTable defined? Use that to find a mapping if possible
Can the child build as AnyCPU? Use that if possible.
Can't determine the platform to pass along, log a warning and undefine platform.

Finally, the SetPlatform metadata is set or undefined per ProjectReference.

Changes Made

MODIFIED TARGETS
- GetTargetFrameworks
  - Extracts $(Platforms) and $(IsVcxOrNativeProj) from ProjectReference Items
NEW TARGETS
- _GetProjectReferencePlatformProperties
  - Calls GetPlatforms on referenced projects
  - Only runs when EnableDynamicPlatformResolution is true on the referencing project
  - Sets or undefines SetPlatform metadata on all referenced projects
New Task
- GetCompatiblePlatform
  - Performs the negotiation
  - Can throw a warning when no compatible platform is found.

Testing

✔️

Notes

Reviewing as a whole is better for this PR.

To Do

Add codes/resources for all warnings
Add tests
Add docs

flow into TargetFramework logic.

…so don't modify any items if the task didn't run or there were no items added to ProjectsWithPlatformAssignment

…existent projects item

…uild. Bring over CanMultiPlatform logic to CrossTargeting.targets

…hoose

…ptions

…atform options" This reverts commit c4a1088.

Continue instead of breaking if the platform lookup table is malformed, other items in the table might be valid. Remove CanMultiPlatform, not necessary if GetNearestPlatformTask can figure that out. Use IsVcxOrNativeProj when setting PlatformLookupTable

…ty already

…PlatformTask. The use case is when a particular project wants its Win32 or AnyCPU platforms to map to a specific other platform

… check when setting default managed->unmanaged PlatformLookupTable

Remove extra file?

…s property already" This reverts commit 6aba0c9.

…ect references

…d project doesn't have Platforms specified

…ild project

…rom a child project" This reverts commit 9b4cbd0.

…s single option

AArnott · 2021-07-27T19:13:19Z

documentation/ProjectReference-Protocol.md

+ Next, every referenced project is required to define a `$(Platforms)` property. `$(Platforms)` is a semicolon-delimited list of platforms that project could build as. `<Platforms>x64;x86;AnyCPU</Platforms>`, for example.
+
+ Lastly, projects that contain `ProjectReference` items may need to define a `$(PlatformLookupTable)` property. `$(PlatformLookupTable)` is a semicolon-delimited list of mappings between projects. `<PlatformLookupTable>win32=x86</PlatformLookupTable>`, for example. This means that if the current project is building for `Win32`, it should build referenced projects using `x86` as the `Platform`. This is mostly relevant for references between managed and unmanaged projects.


The way this paragraph reads makes me wonder if Win32=x86 set in a vcxproj will break P2Ps to other vcxproj's, which expect Win32 and not x86.
How would we resolve this for when a project references a blend of C# and C++ projects?

In this case the PlatformLookupTable is presumably defined globally in some Directory.Build.props, or in the cpp project that could reference c# and cpp projects. In the specific projectreference of cpp -> cpp, you can set PlatformLookupTable (or SetPlatform) metadata on that individual projectreference, and that metadata will take priority.

We could make this a cleaner experience but that would be in a future PR.

AArnott · 2021-07-27T19:14:20Z

documentation/ProjectReference-Protocol.md

@@ -57,7 +57,8 @@ If implementing a project with an “outer” (determine what properties to pass
  * It returns an item with the following metadata:
    * `TargetFrameworks` indicating what TargetFrameworks are available in the project
    * `TargetFrameworkMonikers` and `TargetPlatformMonikers` indicating what framework / platform the `TargetFrameworks` map to.  This is to support implicitly setting the target platform version (for example inferring that `net5.0-windows` means the same as `net5.0-windows7.0`) as well as treating the `TargetFramework` values [as aliases](https://github.com/NuGet/Home/issues/5154)
-    * Boolean metadata for `HasSingleTargetFramework` and `IsRidAgnostic`.
+    * Boolean metadata for `HasSingleTargetFramework` and `IsRidAgnostic` and `IsVcxOrNativeProj`.
+    * `Platforms` indicating what `Platforms` are available for the project to build as.


Nit: the second use of the word platforms is as an english explanation rather than a recursive definition, so drop the identifier treatment.

Suggested change

* `Platforms` indicating what `Platforms` are available for the project to build as.

* `Platforms` indicating what platforms are available for the project to build as.

AArnott · 2021-07-27T19:15:49Z

documentation/ProjectReference-Protocol.md

+| Managed -> Managed | No |  |
+| Unmanaged -> Managed | **Yes** |  |
+| Managed -> Unmanaged | Optional | Uses default mapping: `AnyCPU=Win32;x86=Win32` |


Is this default mapping still correct? I thought we agreed in the discussion that AnyCPU->Win32 should emit a warning at least, if not block, unless the user explicitly opted into that jump.

To clarify here, when jumping from a parent that's AnyCPU to a child that is anything non-AnyCPU, a warning should be emitted>

That sounds good. But with the default table, an AnyCPU->Win32 jump looks permissible. I don't think I would expect a warning if an entry exists in the table. If it does, what would I do to suppress the warning if I needed to?

AArnott · 2021-07-27T19:17:17Z

documentation/ProjectReference-Protocol.md

+| Unmanaged -> Unmanaged | No |  |
+| Managed -> Managed | No |  |
+| Unmanaged -> Managed | **Yes** |  |


Why is a table required when C++ references a C# project? C++ is always arch-specific and C# will almost always have either AnyCPU or a compatible arch-specific platform. Couldn't a default mapping of win32=x86;x64=x64;arm=arm apply here?

My main confusion here was win32 being applicable to x86 and AnyCPU and not being entirely sure if either should be the default mapping from win32. Is win32->x86 more common?

Side note: x64=x64 isn't necessary because that's auto-detected in the task.

It goes back to my TFM analogy: where netstandard2.0 and net472 are both built by a child, a net472 parent project will pick the platform-specific one (net472) because if you offer something portable and a compatible specific target, the only reason the specific target would exist is because it is preferred over the portable one.
So ya, win32->x86 would be preferred over win32->anycpu by the same reasoning.
But of course for the (default) C# project that doesn't list x86 in its Platforms property, then the behavior would be win32->anycpu because of your "fallback to anycpu" logic. So your table for vcxproj->csproj references would just be win32=x86. And all other platform combination either may directly (e.g. x64=x64) or use the *=anycpu fallback.

documentation/ProjectReference-Protocol.md

AArnott · 2021-07-27T19:24:30Z

documentation/ProjectReference-Protocol.md

+```xml
+<ItemGroup>
+  <ProjectReference Include="B.csproj" PlatformLookupTable="Win32=AnyCPU">


Since this should happen automatically in most cases (as per my above comment), a more compelling sample would be a C# -> C++ reference I think. Of course in that case a default table should be able to resolve it without help, except in the case where C# only defines AnyCPU. In that case though, would the easiest thing be for the customer to set the table property, or just directly set the SetPlatform metadata?

When there's only one option, the customer should just set SetPlatform. It's when the parent project could build as multiple platforms and we're not sure which one its building as before the build has started.

Ok, that makes sense. But specifying Win32=AnyCPU here should only be necessary if C# offered an x86 option (that would fit the default mapping table) and the user didn't want that option taken but wanted AnyCPU to be used instead. This would be very strange, as why would the user define an x86 target platform and then not use it in an x86 process?

AArnott · 2021-07-27T19:25:36Z

documentation/ProjectReference-Protocol.md

+```xml
+<ItemGroup>
+  <ProjectReference Include="B.csproj" PlatformLookupTable="Win32=AnyCPU">


Up to this point, the docs keep referring to PlatformLookupTable as a property, but this sample defines it as a ProjectReference item metadata. Is it one, the other, or both? Should the docs clarify this a bit more?

The docs definitely need a refresh 🙂

…ookupTables be opt-out

src/Tasks.UnitTests/GetCompatiblePlatform_Tests.cs

src/Tasks/GetCompatiblePlatform.cs

rainersigwald · 2021-07-27T20:04:50Z

src/Tasks/GetCompatiblePlatform.cs

+
+            foreach (string s in stringTable.Split(MSBuildConstants.SemicolonChar, StringSplitOptions.RemoveEmptyEntries))
+            {
+                string[] keyVal = s.Split(MSBuildConstants.EqualsChar, StringSplitOptions.RemoveEmptyEntries);


What does RemoveEmptyEntries buy you here?

Only unnecessary ~~errors~~ warnings in strange translation tables like A=Z;;B=G that are "technically" correct. I could see table entries added via properties that may or may not have values, but may be easier to include at once for convenience.

For the semicolon split I'm totally on board. For the equals I'm still pretty skeptical.

Sounds reasonable to enforce proper formatting for individual entries. Will allow empty entries on splitting the equals.

Allowing empty entries complicates the invalid check and doesn't get us much benefit. Disallowing empty entries guarantees a=b at minimum, and keeps the invalid check simpler (length <=1 or > 2 then invalid). Otherwise its an extra two string null/empty checks, which would have been handled by string.Split

But it also silently handles super confusing cases like =x86========anycpu==, which I think is bad.

But it also silently handles super confusing cases like =x86========anycpu==, which I think is bad.

Blarg, blarg I say! (Translation: you're right, will fix)

rainersigwald · 2021-07-27T20:13:27Z

documentation/ProjectReference-Protocol.md

@@ -57,7 +57,8 @@ If implementing a project with an “outer” (determine what properties to pass
  * It returns an item with the following metadata:
    * `TargetFrameworks` indicating what TargetFrameworks are available in the project
    * `TargetFrameworkMonikers` and `TargetPlatformMonikers` indicating what framework / platform the `TargetFrameworks` map to.  This is to support implicitly setting the target platform version (for example inferring that `net5.0-windows` means the same as `net5.0-windows7.0`) as well as treating the `TargetFramework` values [as aliases](https://github.com/NuGet/Home/issues/5154)
-    * Boolean metadata for `HasSingleTargetFramework` and `IsRidAgnostic`.
+    * Boolean metadata for `HasSingleTargetFramework` and `IsRidAgnostic` and `IsVcxOrNativeProj`.


Instead of IsVcxOrNativeProj can we give a more crisp meaning? Like "SupportsPlatformNegotiation" or something?

SupportsPlatformNegotiation is a bit too broad for this bool.

IsVcxOrNativeProj is used for:

Should SetPlatform be Platform= or PlatformTarget=?

Should we work around some Target Framework negotiation logic? (because it's conditioned against .vcxproj and .nativeproj)

Are we explicitly looking at a .vcxproj or .nativeproj from a managed project? (non .vcxproj/.nativeproj)

src/Tasks/Microsoft.Common.CurrentVersion.targets

rainersigwald · 2021-07-27T20:19:09Z

src/Tasks/Microsoft.Common.CurrentVersion.targets

+    ======================================================================================
+  -->
+
+  <!-- Managed projects need to have PlatformTarget set for SetPlatform negotiation. Default to $(Platform), which is AnyCPU by default. -->


Managed projects need to have PlatformTarget set for SetPlatform negotiation.

Why?

Actually, do you need this given this below

 <PropertyGroup> <ParentPlatform>$(PlatformTarget)</ParentPlatform> <ParentPlatform Condition="'$(MSBuildProjectExtension)' == '.vcxproj' or '$(MSBuildProjectExtension)' == '.nativeproj'">$(Platform)</ParentPlatform> </PropertyGroup>

Why?

There was a case I hit where PlatformTarget wasn't set when building a managed project.

Actually, do you need this given this below

That's why the above is needed. I suppose we could change it to:

<PropertyGroup> <ParentPlatform>$(PlatformTarget)</ParentPlatform> <ParentPlatform Condition="'$(PlatformTarget)' == '' or '$(MSBuildProjectExtension)' == '.vcxproj' or '$(MSBuildProjectExtension)' == '.nativeproj'">$(Platform)</ParentPlatform> </PropertyGroup>

rainersigwald · 2021-07-27T20:19:46Z

src/Tasks/Microsoft.Common.CurrentVersion.targets

+
+  <Target Name="_GetProjectReferencePlatformProperties"
+          Condition="'$(EnableDynamicPlatformResolution)' == 'true'
+                     and '$(BuildingInsideVisualStudio)' != 'true'


Comment this please

src/Tasks/Microsoft.Common.CurrentVersion.targets

benvillalobos · 2021-07-27T21:39:17Z

Notes from the meeting between @AArnott and @yuehuang010 and myself today:

✔️ We should construct the set of platforms a cpp or nativeproj can build as via its itemgroup labeled ProjectConfigurations, which has the Platform for each configuration.
❌ Whenever we're currently building as AnyCPU and "hop off" to any non-AnyCPU architecture, we should log a warning when it isn't a "user-specified" (lookup table) hop
- On second thought, the only way this could happen is if it came from a lookup table.
✔️ If the current project platform matches the referenced project's platform, that should always take priority.
✔️ AnyCPU should not map to Win32 by default

benvillalobos · 2021-07-28T05:10:21Z

AnyCPU should not map to Win32 by default

@AArnott When a customer opts into this feature and has no PlatformLookupTable, a ProjectReference from a managed project to an unmanaged project won't "just work". You were pushing for a solution that had the feature do most of the work, is requiring a lookup table in this scenario acceptable? The answer must be yes since we agreed AnyCPU shouldn't map to win32 by default, just double checking.

This doesn't change the fact that if A was x86, it would see that B could be x86 and map to that by default.

AArnott · 2021-07-28T14:19:11Z

This doesn't change the fact that if A was x86, it would see that B could be x86 and map to that by default.

We also want a C# x86 project to reference a C++ win32 project without any extra effort. So a default mapping of x86=win32 is desirable.

But yes, I think AnyCPU=win32 is not a good thing. Warn by default for such jumps, and make it easy to suppress the warning (by adding to the lookup table or adding SetPlatform metadata) for those that want that behavior.

This reverts commit f483ff0.

AArnott

Thanks for doing this. This is gonna be awesome for building msbuild projects without a solution configuration!

documentation/ProjectReference-Protocol.md

AArnott · 2021-07-29T13:03:43Z

documentation/ProjectReference-Protocol.md

+If only set in one project, the `SetPlatform` metadata will carry forward to every consecutive project reference.
+
+Next, every referenced project is required to define a `Platforms` property, where `Platforms` is a semicolon-delimited list of platforms that project could build as. For `.vcxproj` or `.nativeproj` projects, `Platforms` is constructed from the `ProjectConfiguration` items that already exist in the project. For managed SDK projects, the default is `AnyCPU`. Managed non-SDK projects need to define this manually.


Platforms is constructed from the ProjectConfiguration items that already exist in the project

Is this done at execution time instead of evaluation time then? Just curious because items can't usually be used to create properties.

Is this done at execution time instead of evaluation time then?

Right, because it's running in the GetTargetFrameworks target.

documentation/ProjectReference-Protocol.md

benvillalobos added 30 commits July 6, 2021 10:59

GetPlatforms target added

68c396b

Add _GetProjectReferencePlatformProperties

8b85f9e

Add GetNearestPlatformTask v1

2c8c91f

Changes to GetNearestPlatformTask, Platform items

e4f95be

flow into TargetFramework logic.

Flip boolean logic for setting SetPlatform metadata

2edda9e

Check for comma to see if project can multiplatform

e5d4f9c

Don't run GetNearestPlatformTask if a project can't multiplatform. Al…

42cf4e1

…so don't modify any items if the task didn't run or there were no items added to ProjectsWithPlatformAssignment

Combine itemgroup logic when transferring items with new metadata to …

8e39ff1

…existent projects item

Remove redundant CanMultiPlatform check

e4c25a0

Only account for $(Platforms) when retrieving possible platforms to b…

92f329c

…uild. Bring over CanMultiPlatform logic to CrossTargeting.targets

Undefine Platform/PlatformTarget when there is no clear platform to c…

e036043

…hoose

Add property to simplify vcxproj and nativeproj checks

cc9a401

PlatformOptions should be semicolon-delimited

4f6e41f

Take Platform or PlatformTarget into account when consider platform o…

0cd0e52

…ptions

Revert "Take Platform or PlatformTarget into account when consider pl…

ed3578e

…atform options" This reverts commit c4a1088.

Code cleanup.

1351751

Continue instead of breaking if the platform lookup table is malformed, other items in the table might be valid. Remove CanMultiPlatform, not necessary if GetNearestPlatformTask can figure that out. Use IsVcxOrNativeProj when setting PlatformLookupTable

Refactor GetNearestPlatformTask

3161d84

Use BuildArchitecturesAllowed as PlatformOptions. VS Uses this proper…

b6d7aee

…ty already

Account for user-defined PlatformLookupTables when calling GetNearest…

649cc5b

…PlatformTask. The use case is when a particular project wants its Win32 or AnyCPU platforms to map to a specific other platform

Add verbose logging to help debug

9594068

Pass parent platform based on file extension. Add 'is parent managed'…

5b2ba53

… check when setting default managed->unmanaged PlatformLookupTable

Clean up DefaultToVcxPlatformMapping

9dae683

Remove extra file?

Revert "Use BuildArchitecturesAllowed as PlatformOptions. VS Uses thi…

84b7d71

…s property already" This reverts commit 6aba0c9.

Only run _GetProjectReferencePlatformProperties if there are any proj…

890bed2

…ect references

Add warning when we want to dynamically figure out platform, but chil…

d5a65fb

…d project doesn't have Platforms specified

Undefine Platform and PlatformTarget when getting platforms from a ch…

097cb71

…ild project

Revert "Undefine Platform and PlatformTarget when getting platforms f…

8030f10

…rom a child project" This reverts commit 9b4cbd0.

Move GetPlatforms target for easier copy/paste

2841da2

Only warn on $(Platforms) if it's not going to skip the platform logic

7ff6614

Remove defaulting to the single option when $(Platforms) only contain…

0ad8a91

…s single option

Matching platforms takes priority over AnyCPU

a991ee0

AArnott reviewed Jul 27, 2021

View reviewed changes

benvillalobos added 2 commits July 27, 2021 13:06

Add default cpp to managed PlatformLookupTable. Let default PlatformL…

f4e8c35

…ookupTables be opt-out

Remove AnyCPU to Win32 default mapping

2e7d06f

rainersigwald reviewed Jul 27, 2021

View reviewed changes

benvillalobos added 9 commits July 27, 2021 14:45

Matching platforms takes #1 priority

429d097

Construct Platforms via ProjectConfiguration for cpp projects

b1eac04

PR Feedback: string.empty, extra keyval check, shouldly

4ffc4da

PR Feedback: Clearer comments and low importance messages

9a938d7

Reword 'parent' to 'current'

5530520

Reword 'child' to 'projectreference'

56b466f

Code cleanup, add resources for low prio messages

d4fd1fa

Enable nullable. Marked required params as required

f9c3e66

Trim whitespace around translation table and each entry

1dbf392

Change resource to be more specific about what 'normally' means

d011e32

Allow empty entries when splitting individual mappings

f483ff0

benvillalobos force-pushed the setplatform-negotiation branch from 0018e55 to f483ff0 Compare July 28, 2021 15:36

benvillalobos added 3 commits July 28, 2021 09:10

Revert "Allow empty entries when splitting individual mappings"

9486435

This reverts commit f483ff0.

Update SetPlatform documentation

9404a0c

dont clear empty entries on equals split. simplify length check

275738f

AArnott approved these changes Jul 29, 2021

View reviewed changes

PR Feedback: Doc clarification

3cb5a72

MarkOsborneMS approved these changes Jul 29, 2021

View reviewed changes

benvillalobos added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jul 29, 2021

rainersigwald approved these changes Jul 30, 2021

View reviewed changes

marcpopMSFT merged commit b6e7d60 into dotnet:main Jul 30, 2021

benvillalobos mentioned this pull request Sep 24, 2021

ProjectReference Platform Negotiation - Pass Platform Instead of PlatformTarget #6887

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ProjectReferences Negotiate SetPlatform Metadata #6655

ProjectReferences Negotiate SetPlatform Metadata #6655

benvillalobos commented Jul 6, 2021 •

edited

AArnott Jul 27, 2021

benvillalobos Jul 27, 2021

AArnott Jul 27, 2021

AArnott Jul 27, 2021

benvillalobos Jul 27, 2021

AArnott Jul 27, 2021

AArnott Jul 27, 2021

benvillalobos Jul 27, 2021 •

edited

AArnott Jul 27, 2021

AArnott Jul 27, 2021

benvillalobos Jul 27, 2021

AArnott Jul 27, 2021

AArnott Jul 27, 2021

benvillalobos Jul 27, 2021

rainersigwald Jul 27, 2021

benvillalobos Jul 28, 2021 •

edited

rainersigwald Jul 28, 2021

benvillalobos Jul 28, 2021 •

edited

benvillalobos Jul 28, 2021 •

edited

rainersigwald Jul 28, 2021

benvillalobos Jul 28, 2021

rainersigwald Jul 27, 2021

benvillalobos Jul 28, 2021 •

edited

rainersigwald Jul 27, 2021

rainersigwald Jul 27, 2021

benvillalobos Jul 27, 2021 •

edited

rainersigwald Jul 27, 2021

benvillalobos commented Jul 27, 2021 •

edited

benvillalobos commented Jul 28, 2021 •

edited

AArnott commented Jul 28, 2021

AArnott left a comment

AArnott Jul 29, 2021

benvillalobos Jul 29, 2021

		Next, every referenced project is required to define a `$(Platforms)` property. `$(Platforms)` is a semicolon-delimited list of platforms that project could build as. `<Platforms>x64;x86;AnyCPU</Platforms>`, for example.

		Lastly, projects that contain `ProjectReference` items may need to define a `$(PlatformLookupTable)` property. `$(PlatformLookupTable)` is a semicolon-delimited list of mappings between projects. `<PlatformLookupTable>win32=x86</PlatformLookupTable>`, for example. This means that if the current project is building for `Win32`, it should build referenced projects using `x86` as the `Platform`. This is mostly relevant for references between managed and unmanaged projects.

	* `Platforms` indicating what `Platforms` are available for the project to build as.
	* `Platforms` indicating what platforms are available for the project to build as.

		If only set in one project, the `SetPlatform` metadata will carry forward to every consecutive project reference.

		Next, every referenced project is required to define a `Platforms` property, where `Platforms` is a semicolon-delimited list of platforms that project could build as. For `.vcxproj` or `.nativeproj` projects, `Platforms` is constructed from the `ProjectConfiguration` items that already exist in the project. For managed SDK projects, the default is `AnyCPU`. Managed non-SDK projects need to define this manually.

ProjectReferences Negotiate SetPlatform Metadata #6655

ProjectReferences Negotiate SetPlatform Metadata #6655

Conversation

benvillalobos commented Jul 6, 2021 • edited

Context

Changes Made

Testing

Notes

To Do

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

benvillalobos Jul 27, 2021 • edited

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

benvillalobos Jul 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benvillalobos Jul 28, 2021 • edited

Choose a reason for hiding this comment

benvillalobos Jul 28, 2021 • edited

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

benvillalobos Jul 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benvillalobos Jul 27, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benvillalobos commented Jul 27, 2021 • edited

benvillalobos commented Jul 28, 2021 • edited

AArnott commented Jul 28, 2021

AArnott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benvillalobos commented Jul 6, 2021 •

edited

benvillalobos Jul 27, 2021 •

edited

benvillalobos Jul 28, 2021 •

edited

benvillalobos Jul 28, 2021 •

edited

benvillalobos Jul 28, 2021 •

edited

benvillalobos Jul 28, 2021 •

edited

benvillalobos Jul 27, 2021 •

edited

benvillalobos commented Jul 27, 2021 •

edited

benvillalobos commented Jul 28, 2021 •

edited