Skip to content

Conversation

@Anipik
Copy link
Contributor

@Anipik Anipik commented Dec 18, 2020

Bug

Fixes: NuGet/Home#10396
Regression: No

Fix

Details: Currently we are using the IncludeBuildOutput Variable as on outerbuild parameter to include the build output for the tfms. Adding it as condition on innerbuild target helps it to add includeOutput for specific tfms.

Testing

Tests Added: Yes

@Anipik Anipik requested a review from a team as a code owner December 18, 2020 02:52
@donnie-msft donnie-msft added the Community PRs created by someone not in the NuGet team label Dec 18, 2020
@nkolev92
Copy link
Member

nkolev92 commented Dec 21, 2020

@Anipik Can you please keep the PR template?

Can you please add tests in https://github.com/NuGet/NuGet.Client/blob/dev/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/PackCommandTests.cs.

Also make sure you see my latest comment in the issue.

@Anipik
Copy link
Contributor Author

Anipik commented Dec 21, 2020

@nkolev92 i added the test and template.

Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT

@nkolev92
Copy link
Member

Thanks @Anipik.

I'd like to wait for @zkat to be back from vacation before we merge this.

@nkolev92 nkolev92 self-assigned this Dec 22, 2020
@nkolev92 nkolev92 requested a review from zkat December 22, 2020 00:44
@nkolev92
Copy link
Member

nkolev92 commented Jan 6, 2021

@Anipik

The build failed because of formatting issues:

test\NuGet.Core.FuncTests\Dotnet.Integration.Test\PackCommandTests.cs(3710,135): Fix whitespace formatting.

Can you please fix the formatting?

Our instructions were recently updated with that: https://github.com/NuGet/NuGet.Client/blob/dev/CONTRIBUTING.md#build.

@zkat

Happy with this change?

Copy link
Contributor

@erdembayar erdembayar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix formatting I commented on line.

@Anipik
Copy link
Contributor Author

Anipik commented Jan 12, 2021

@nkolev92 @erdembayar i pushed the change yesterday. It seems like CI is stuck

@nkolev92
Copy link
Member

CI doesn't runt automatically for external forks.

I've kicked it off again

@Anipik
Copy link
Contributor Author

Anipik commented Jan 12, 2021

@nkolev92 the ci is green, can we go ahead and merge this one ?

@Anipik
Copy link
Contributor Author

Anipik commented Jan 14, 2021

cc @erdembayar for the final merge

@erdembayar
Copy link
Contributor

cc @erdembayar for the final merge

@zivkan

Can you take a look here? It looks it's ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PRs created by someone not in the NuGet team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce a new msbuild property to exclude build output for specific tfms during pack task

4 participants