-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include user specified MSBuild parameters when evaluating MSBuild properties #352
Conversation
…arameters include user specified msbuild parameters.
47d76ee
to
cac10d2
Compare
@@ -211,9 +211,10 @@ public static string DeterminePublishLocation(string workingDirectory, string pr | |||
/// Looks up specified properties from a project. | |||
/// </summary> | |||
/// <param name="projectLocation">The location of the project file.</param> | |||
/// <param name="msBuildParameters">Additonal MSBuild paramteres passed by the user from the commandline</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// <param name="msBuildParameters">Additonal MSBuild paramteres passed by the user from the commandline</param> | |
/// <param name="msBuildParameters">Additional MSBuild parameters passed by the user from the commandline</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks fixed
[InlineData("TargetFramework", "", "net6.0")] | ||
[InlineData("TargetFramework", "/p:NonExistence=net20.0", "net6.0")] | ||
[InlineData("TargetFramework", "/p:TargetFramework=net20.0", "net20.0")] | ||
public void TestPropertyEvaluationWithMSBuildParameters(string property, string msbuildparameters, string expectedValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth adding a test case for when more that one property is added by the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple more test scenarios.
Issue #, if available:
#351
Description of changes:
In the recent release of Amazon.Lambda.Tools we switched to using
dotnet msbuild getproperty
command to evaluate MSBuild properties likePublishAot
. It is possible for users to specify additional MSBuild parameters via the--msbuild-parameters
switch that might affect the evaluation. That is the case of the linked issue.This PR pass along the user specified value for
--msbuild-parameters
to thedotnet msbuild getproperty
command so the evaluation has the full context for evaluation.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.