Closed AndyGerlicher closed 7 years ago
@AndyGerlicher, Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request. Thanks, .NET Foundation Pull Request Bot
I reverted the change to Microsoft.NuGet.Solution.ImportAfter.targets
. Not fully clear on where that would be needed, but after talking with @emgarten we think it's something to do with sln restore and I suspect it's imported when Common targets are not. So we need to leave as-is. Previously if the Common targets were imported, this would have resulted in a double import warning. With my changes, Common targets is still responsible for importing (directly instead of indirectly) so there should be no net change.
@tmeschter @emgarten This was merged into VS just now. Please merge here as well.
@tmeschter which branch should this go into? Which is the most recent?
The “dev” branch.
-Tom
From: Justin Emgarten [mailto:notifications@github.com] Sent: Sunday, October 29, 2017 3:49 PM To: NuGet/NuGet.BuildTasks NuGet.BuildTasks@noreply.github.com Cc: Tom Meschter Tom.Meschter@microsoft.com; Mention mention@noreply.github.com Subject: Re: [NuGet/NuGet.BuildTasks] Remove ImportAfter for NuGet.targets (#44)
@tmeschterhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftmeschter&data=02%7C01%7Ctom.meschter%40microsoft.com%7C92cbc0c959854b3338c208d51f1f2fc0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636449141163654685&sdata=rRPmmSWSxj5xPf1hDsdwdK3gEAd3eQ%2F0vtHAmaH2QvQ%3D&reserved=0 which branch should this go into? Which is the most recent?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FNuGet%2FNuGet.BuildTasks%2Fpull%2F44%23issuecomment-340309583&data=02%7C01%7Ctom.meschter%40microsoft.com%7C92cbc0c959854b3338c208d51f1f2fc0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636449141163654685&sdata=f96te03a%2BBpr0IbdffIxburAncX%2FWeaEg286iFnLn78%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAKBR6jsPHoipyspHfE7oOS07zhdtwTQnks5sxQDCgaJpZM4QE5Cd&data=02%7C01%7Ctom.meschter%40microsoft.com%7C92cbc0c959854b3338c208d51f1f2fc0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636449141163654685&sdata=ak63mDOaZI72qtzhMiRwnVHqJSx2uKy8PVBsWyR7vvE%3D&reserved=0.
This would be taken in conjunction with https://github.com/Microsoft/msbuild/pull/2663
Wasn't sure what to do about
Microsoft.NuGet.Solution.ImportAfter.targets
. I think it wouldn't be needed any longer, but I didn't see anything referring to the file. So let me know what I should do about that if we are going to take this. This and the MSBuild change will need to go together to CLI and VS to avoid double import warning.