Centrally manage dependencies and its versions #96
Conversation
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
3 times, most recently
from
04a06d1 to
5767d37
Compare
There was a problem hiding this comment.
I know this is work in progress, but please don't have these changes based on top of the other branch, let's keep PRs smaller. I'm saying this because I can't say when/if that base PR will get merged, so I don't think it's worth it to block this until that one is resolved, as it's unrelated 🙂
Thanks!
|
There's a change in how we pack the SourceGen project into the parent project in this patch. I hope to create a new PR for that change but I will test it here before I do that. That change was made in response to this feature. I don't want to update the workaround just to unblock the PR. This patch can wait as it needs support from VS IDE. As far as I know, v17.2 is the target to enable central package versioning in the NuGet PM UI. |
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
from
5767d37 to
b4fdefe
Compare
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
2 times, most recently
from
3369aef to
ae8500c
Compare
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
5 times, most recently
from
cd49f35 to
bfe8944
Compare
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
2 times, most recently
from
ba97f5e to
b272630
Compare
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
from
b272630 to
f0988ed
Compare
There was a problem hiding this comment.
The PR is doing more than it states. Even if you leave off the first three commits which are part of your refactor. You're tackling two different issues
- Building to a common path (a large portion of this PR)
- Moving to centrally managed package versions
I think you should get feedback on 1. before implementing it as changing the build path to something you want is subjective. What's wrong with the current system?
I've seen a lot of Microsoft repos have a common output that falls under artifacts/. Pretty much anything using the Arcade SDK. I've never seen a convention of outputting to ~build and ~packages (I searched across GitHub and only found your personal projects using it).
artifacts/bin, artifacts/obj, artifacts/packages, etc..
|
Initially, I wanted just two PRs, one for refactoring and another for central versioning but MVVM Changes are important enough to warrant an issue and PR of their own. But I can create a separate PR for common paths, if needed. We can do
|
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
4 times, most recently
from
d3a9294 to
f612708
Compare
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
from
f612708 to
2cd3932
Compare
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
4 times, most recently
from
0829810 to
4f5f5c3
Compare
- Rename `build` folder to `eng`:
- This is a standard build infra directory used in official dotnet projects.
- Rename NuGet Icon to `Icon.png`:
- This is no longer used as a public reference point for NuGet icon URL.
- Also, Icon URL is deprecated. Hence, it's safe to change.
- Normalize casing for `ReadMe.md`:
- Repository information files such as ReadMe, License, etc... are only UPPER_CASE
if they are without an extension. With extension, the casing becomes PascalCase
or Kebab-Case. The primary reason is attention to the presentation of file names.
- Do Kebab-Case when a phrase is presented. E.g., `Code-of-Conduct.md`.
- Rename solution file to `CommunityToolkit.sln`:
- The `dotnet` seems implied and also doesn't stand-out in the file list because of the lower casing and `d` char.
- Spaces are a main issue when doing automation (_like using `*.sln` in build scripts and in URLs it adds `%20`_).
- Move `toolkit.snk` file to `eng` sub-directory.
- Remove un-needed and deleted files from solution.
- Update Git Ignore entries to latest from upstream.
- Indent text in `ThirdPartyNotices.txt` with spaces instead.
- Fix the text flow warping in the MSBuild Console logging. - Use checked version properties instead of hard-coded checks. - Update the structure of the projects list in the Solution file. - Refactor Roslyn multi-targeting to use multiple projects in the same project directory without using Shared projects. This refactoring is made in preparation for the solution to use the NuGet's CPVM (Central Package Versions Management) feature. This also slightly improves IDE load time and Build performance.
- Move the T4 MSBuild logic to a new file.
- Move Public Key to a new file 'toolkit.spk'.
- Move MSBuild logic to near-by `Directory.Build.{props|targets}` files.
Follow the following rules: - SOF (_Start Of File_) Imports, - Core properties, Package properties, build properties, - Build items, Resource items, Content items, Misc. items, - In-place Imports, Choose (_elements within follows outer sort order_) - `ProjectReference` items, `PackageReference` items, - Targets and Properties and Items close to said Targets, - EOF (_End Of File_) Imports. Where there's a condition by target properties, we should group them together under `Choose`. For multiple target values, we should sort them by the order in which they are defined. And the order in which they should be defined is either ascending or descending in terms of compatibility layering.
- Add necessary guard to check for pack. - Remove redundant properties and values. - Remove and adjust quotes in property functions. - Use wildcards to generalize and reduce items declared.
- Remove redundant comments. - Add missing comments on certain code blocks. - Format code in comments with proper quote style.
Some property groups have conditions that also self-explain their purpose. So, Add labels to bare property groups only to differentiate among themselves. Then, when contributors add any additional properties, they'll know where to put them.
When the '$(IncludeContentInPack)' property is false, files specified via '@(None)', '@(Content)' items are excluded from the NuGet package. Adding '@(PackageFile)' items directly to '%(_PackageFiles)' item via the following target ensures that they will always be included in the package. So, Use '%(PackageFile)' item to always include files in the package. By default, they are included in the root of the package but can be overridden via '%(PackageFile.TargetPath)' metadata.
Previously, static output path to the MVVM SourceGen assembly was used to pack the MVVM project. This leads to error when OutputPath was updated dynamically when testing or in forks. So, here, we'll update the build so that the SourceGen build outputs will be dynamically packed.
- MSBuild Item update logic within target broke during 17.3-17.4! Previous logic referencing direct Item names worked fine before, but now needs a proxy/temporary item in order to process the includes. Same with the undefined Metadata, previously it returned empty string for items with the metadata undefined but now throws error. This may be correct behavior for items under target but this difference hinders sharing logic within and out of targets. This has a side-effect of needing to specify fully qualified name "%(Item.Metadata)" which makes it verbose.
The new Output paths point to...
Build : `build\{bin,obj}`
Pack : `packages`
Publish : `publish`
Restore : `build\ext`
Test : `build\TestResults`
with-in the repository root directory.
Manage Packages Centrally using NuGet's CPVM feature. This uses `Directory.Packages.props` to store the package versions in one file.
- Features: Strict Setting this ensures that we won't fall into compiler bugs when using latest features. - LangVersion: Latest Setting this ensures that we always compile against latest version against the using SDK.
Nirmal4G
force-pushed
the
feature/nuget-cpvm
branch
from
4f5f5c3 to
3bdae22
Compare
Info
Fixes CommunityToolkit/WindowsCommunityToolkit#4180
Extracted from CommunityToolkit/WindowsCommunityToolkit#4181
Changes
Know more about NuGet's Central Package Version Management (CPVM) feature.
Currently, NuGet package references are spread across project files. It is difficult to track versions and upgrade them properly. This patch enables NuGet's upcoming feature to manage package versions centrally using
Directory.Packages.propswith the following item spec to store a package version.PR Checklist
featurebranch in your forkNotes
Merge pull request #xxxx from repo/branch, and commit message to either PR message or messages of individual commits. Theauto-mergeoption does this by default.