Skip to content

Conversation

@jeffkl
Copy link
Contributor

@jeffkl jeffkl commented Jan 8, 2024

Bug

Fixes: NuGet/Home#11834

Regression? Last working version:

Description

This change makes it so the presence of a Directory.Packages.props means that central package management is implicitly on. I've had countless people ask over the last year why you have to have the file and explicitly set ManagePackageVersionsCentrally to true. The logic now assumes if Directory.Packages.props exists then ManagePackageVersionsCentrally should be true unless the user has already set that property to a value, including if its set in Directory.Packages.props. This will make it easier for customers to onboard.

This is a redo of #5540 which was closed during the holiday break but then I forced pushed a rebase which GitHub then prevents me from re-opening.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@jeffkl jeffkl self-assigned this Jan 8, 2024
@jeffkl jeffkl requested a review from a team as a code owner January 8, 2024 17:38
@jeffkl jeffkl merged commit c9b81ab into dev Jan 9, 2024
@jeffkl jeffkl deleted the dev-jeffkl-implicit-manage-package-versions-centrally branch January 9, 2024 19:17
@zivkan zivkan mentioned this pull request Jan 15, 2024
8 tasks
@ViktorHofer
Copy link
Contributor

Note that this broke dotnet/runtime#98476 when upgrading to .NET 9 Preview 1. This is because previously an empty Directory.Packages.props file was created to avoid importing a root one. We understand the right fix but it might be worthwhile to write a breaking change notice for this.

@jeffkl
Copy link
Contributor Author

jeffkl commented Feb 27, 2024

Note that this broke dotnet/runtime#98476 when upgrading to .NET 9 Preview 1. This is because previously an empty Directory.Packages.props file was created to avoid importing a root one. We understand the right fix but it might be worthwhile to write a breaking change notice for this.

Sorry about that. Unfortunately its to be expected any repository that's running tests where those tests are going to import the repo's build logic and the repo is using CPM but the tests are not. We really wanted to reduce the friction with onboarding but knew that a couple repos would need to update to explicitly disable CPM for their tests.

jeffkl added a commit that referenced this pull request Jun 27, 2024
jeffkl added a commit that referenced this pull request Jun 27, 2024
jeffkl added a commit that referenced this pull request Jun 27, 2024
jeffkl added a commit that referenced this pull request Jun 28, 2024
jeffkl added a commit that referenced this pull request Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DCR]: Central Package Management - Respect .props file as a way to opt-in to the feature.

5 participants