Add explanation for Builder class in example #15
Conversation
When implementing MSBuildLocator I originally called `MSBuildLocator.RegisterInstance()`, then later in the same function I called other Microsoft.Build functions. This is my first large project with a major C# component. I didn't know that all of the dll imports in my function would be loaded when the function was entered, instead of when the import procedures were called. This request adds a short description above the Builder class to explain why it's necessary--so that the MSBuildLocator dll loads first, then after the appropriate setup is requested and the `Builder` class is instantiated can more Microsoft.Build dlls be loaded properly.
There was a problem hiding this comment.
This is an extremely confusing and subtle requirement of this library. Commenting it thoroughly in the sample is a great idea, thanks!
Did you find that the error added in #14 helped you, or did you have to slog through this yourself to figure this out?
samples/BuilderApp/Program.cs
Outdated
| @@ -101,6 +101,8 @@ private static void Usage() | |||
| } | |||
| } | |||
|
|
|||
| // Class for performing the project build | |||
| // The Microsoft.Build dlls needed from within aren't loaded until after the class is instantiated | |||
There was a problem hiding this comment.
It's actually more fine-grained. Assemblies get loaded just in time to call the first method that uses them. I tried to find documentation on this, but didn't find anything official. This StackOverflow answer quotes a book explanation:
In the CLR, loading typically is triggered by the just in time (JIT) compiler based on types. When the JIT compiler tries to convert a method body from CIL to machine code, it needs access to the type definition of the declaring type as well as the type definitions for the type's fields.
Can you update this to something like
The Microsoft.Build namespaces must be referenced from a method that is called after RegisterInstance so that it has a chance to change their load behavior. Here, we put them into a separate class.
samples/BuilderApp/Program.cs
Outdated
| @@ -101,6 +101,8 @@ private static void Usage() | |||
| } | |||
| } | |||
|
|
|||
| // Class for performing the project build | |||
| // The Microsoft.Build dlls needed from within aren't loaded until after the class is instantiated | |||
There was a problem hiding this comment.
Also, a minor nit: the "C#-y" way to comment this would be in the form of an XML doc comment, something like
/// <summary>
/// Class for performing the project build
/// </summary>
/// <remarks>
/// This is in a separate class because ...
/// </remarks>
|
Alright, comment changes implemented. The error in #14 helped a lot, yeah. It was what spurred me to come look at the example 😄 |
When implementing MSBuildLocator I originally called
MSBuildLocator.RegisterInstance(), then later in the same function I called other Microsoft.Build functions.This is my first large project with a major C# component. I didn't know that all of the dll imports in my function would be loaded when the function was entered, instead of when the import procedures were called.
This request adds a short description above the Builder class to explain why it's necessary--so that the MSBuildLocator dll loads first, then after the appropriate setup is requested and the
Builderclass is instantiated can more Microsoft.Build dlls be loaded properly.