Why using Directory.Packages.props for non CPM related settings

[Mik4sa]

Recently TreatWarningsAsErrors (#440) and TargetFramework (#481) moved into Directory.Packages.props. While I can totally understand why it‘s now configured at a central place I‘m unsure why it moved to the CPM related file. Isn‘t this something that should be located in Directory.Build.props? Or am I missing something here?

In my opinion the Packages file should only contain CPM related stuff while the Build file should contain everything else.

[ardalis]

Yeah, that's a good catch. We should separate them. Honestly I forgot there were 2 directory.*.props files... (and it would actually be nice, IMO, if there were just one but given that there are two and each one has its own responsibilities I agree we should split them out).

[Mik4sa]

Perfect :) I can make a PR in the next days. Should ManagePackageVersionsCentrally also move to Build.props or should it stay there? I could argue for both

Build, because it’s just a property as everything else which basically just enables CPM for every project. The Packages file would then just contain packages and nothing else

Packages, because it’s related to CPM which aligns with the documentation of CPM

[ardalis]

I don't have a strong preference. If there's a standard, officially or de facto, we can use that. Absent one, I'd say put it in Build (and having it will force the existence of Packages).

[Mik4sa]

If there's a standard, officially or de facto, we can use that.

Central Package Management | Microsoft Learn says:

Enabling Central Package Management

To get started with central package management, you must create a Directory.Packages.props file at the root of your repository and set the MSBuild property ManagePackageVersionsCentrally to true.

Inside, you then define each of the respective package versions required of your projects using <PackageVersion /> elements that define the package ID and version.

I don't feel like this is a standard or a recommended way, so I 'd put it in Build to. I'm going to start with the PR now.

[KyleMcMaster]

I do agree all the other build properties like TargetFramework should belong in Directory.Build.props.

But unless I am reading this section of the Central Package Management docs incorrectly, I believe the ManagePackageVersionsCentrally attribute should be in Directory.Packages.props based on this example snippet provided in the documentation.

Directory.Packages.props:

<Project>
  <PropertyGroup>
    <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
  </PropertyGroup>
  <ItemGroup>
    <PackageVersion Include="Newtonsoft.Json" Version="13.0.1" />
  </ItemGroup>
</Project>

[Mik4sa]

@KyleMcMaster This is exactly what I meant. But still I think this is no enforcement and no recommendation nor best practice. Logically this just sets a property for every project and such things belong into the Directory.Build.props IMO.
It can stay in Directory.Packages.props if the majority/@ardalis decides so. I'm going to edit my PR then.

[ardalis]

Based on my read of the docs I think if we were only doing package dependencies at the folder level I'd put the directive in the packages file, but since we have a Build props file anyway I think it makes sense to put it there and just have the package listing in the packages file.

[KyleMcMaster]

@ardalis Makes sense! 👍