Skip to content

Stop paying attention to PatchConfig.props #16748

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

Merged
merged 2 commits into from
Nov 19, 2019
Merged

Stop paying attention to PatchConfig.props #16748

merged 2 commits into from
Nov 19, 2019

Conversation

dougbu
Copy link
Contributor

@dougbu dougbu commented Nov 1, 2019

  • part of aspnet/AspNetCore-Internal#3024

Summary

The ASP.NET Core team has hit repeated issues in servicing with merging fixes and not shipping the affected packages. 3.0 and later branches ship far fewer packages than 2.x and we have no (LZMA) downsides to shipping everything. The ASP.NET Core team thus decided to simplify the infrastructure and remove the previous ship / no-ship controls (in PatchConfig.props). This PR does the minimum change to simply ignore that file completely removes the PatchConfig.props file and all references to it.

We will probably follow up in 2.x branches to change the approach, automating what ships more extensively and removing the need for manual PatchConfig.props changes. That is not this change.

Small exception

"Everything" does not include the targeting packs or Microsoft.AspNetCore.App.Ref. We do not normally service those.

Customer Impact

Never miss a fix after a servicing upgrade

Regression?

No

Testing

PR validation build includes stable versioning and published everything. This can be observed in the Source_Build_Packages and Windows_Packages listings at https://dev.azure.com/dnceng/public/_build/results?buildId=415240&view=artifacts

Small exception

Microsoft.AspNetCore.App.Ref and the targeting packs were produced in that build. That's due to the 3.0.1 special case.

Risk

Low.

@dougbu dougbu requested a review from a team November 1, 2019 17:08
@dougbu dougbu added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Nov 1, 2019
Copy link
Contributor

@JunTaoLuo JunTaoLuo left a comment

Choose a reason for hiding this comment

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

I don't think this build will have any effect though since the branch doesn't have stabilized build set to true.

@dougbu dougbu added this to the 3.0.2 milestone Nov 4, 2019
@dougbu
Copy link
Contributor Author

dougbu commented Nov 4, 2019

I don't think this build will have any effect though since the branch doesn't have stabilized build set to true.

Agreed. I rebased on current 'release/3.0' to pick up stabilization setting.

@dougbu dougbu added the ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. label Nov 9, 2019
@Pilchie Pilchie added the Servicing-consider Shiproom approval is required for the issue label Nov 11, 2019
@dougbu
Copy link
Contributor Author

dougbu commented Nov 12, 2019

Fired off a new additional test build at https://dev.azure.com/dnceng/internal/_build/results?buildId=423518&view=results

@dougbu
Copy link
Contributor Author

dougbu commented Nov 12, 2019

/bump reviewers
(All checks successful, including the test internal build.)

@dougbu
Copy link
Contributor Author

dougbu commented Nov 12, 2019

Asking for another review because this is no longer a one-liner

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. labels Nov 12, 2019
@dougbu dougbu merged commit 7891c83 into release/3.0 Nov 19, 2019
@dougbu dougbu deleted the dougbu-patch-1 branch November 19, 2019 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants