Skip to content

Update package titles and consolidate build logic#3894

Merged
michael-hawker merged 12 commits intoCommunityToolkit:mainfrom
Nirmal4G:feature/refactor
Jul 20, 2021
Merged

Update package titles and consolidate build logic#3894
michael-hawker merged 12 commits intoCommunityToolkit:mainfrom
Nirmal4G:feature/refactor

Conversation

@Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented Mar 26, 2021 •
edited
Loading

Fixes #3898

  1. Consolidate common MSBuild logic
  2. Organize/Consolidate Package tags
  3. Update Package titles and description
  4. Update solution to properly pack design tools
  5. Move in-package icon to root and rename to Icon.png (matching "official" dotnet packages).

Also Fixes #3897

PR Type

What kind of change does this PR introduce?

  • Code style update (formatting)
  • Refactoring (no functional changes, no API changes)
  • Build related changes

What is the current behavior?

It's hard to read and understand the repo (because of the redundant files and logic). Package titles are messy. Packages doesn't contain DesignTools plugins.

What is the new behavior?

It's now little bit better to read and understand the repo. Update Package titles to follow common (machine readable too) format. Packages now contain DesignTools plugins.

PR Checklist

Please check if your PR fulfils the following requirements:

  • Tested code with current supported SDKs
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

  • If you're editing this patch tree, please rebase on latest HEAD and then commit, without updating from the latest HEAD.
  • When merging, please update the commit title to PR title instead of the default Merge pull request #xxxx from repo/branch, and commit message to either PR message or messages of individual commits. The auto-merge bot does this by default.

@ghost
Copy link

ghost commented Mar 26, 2021

Thanks Nirmal4G for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested review from Kyaa-dost, Rosuavio, azchohfi and michael-hawker March 26, 2021 19:15
@Nirmal4G Nirmal4G changed the title Refactor project files Refactor project files and fixup markdown files Mar 26, 2021
@Nirmal4G Nirmal4G changed the title Refactor project files and fixup markdown files Update package titles and markdown files Mar 27, 2021
@Nirmal4G Nirmal4G changed the title Update package titles and markdown files Update package titles and refactor markdown files Mar 27, 2021
@Nirmal4G Nirmal4G changed the title Update package titles and refactor markdown files Update package titles, format markdown and consolidate build logic Mar 27, 2021
@Nirmal4G Nirmal4G force-pushed the feature/refactor branch 3 times, most recently from 021c9dd to b60b7de Compare March 27, 2021 14:25
@Nirmal4G Nirmal4G changed the title Update package titles, format markdown and consolidate build logic Update package titles and consolidate build logic Mar 27, 2021
@ghost ghost added the improvements ✨ label Apr 3, 2021
@Nirmal4G Nirmal4G force-pushed the feature/refactor branch 2 times, most recently from 34b3c01 to 4a74699 Compare May 5, 2021 09:44
@Nirmal4G Nirmal4G force-pushed the feature/refactor branch 2 times, most recently from 5a28070 to f06b480 Compare May 17, 2021 20:20
This was referenced May 17, 2021
Merged
Open
@michael-hawker
Copy link
Member

michael-hawker commented May 20, 2021

This looks to be built on #3893, so waiting for that to be finished before we look at this one.

@Nirmal4G Nirmal4G marked this pull request as ready for review June 16, 2021 14:51
@Nirmal4G Nirmal4G mentioned this pull request Jun 16, 2021
Open
michael-hawker
michael-hawker reviewed Jun 16, 2021
View reviewed changes
@Nirmal4G Nirmal4G force-pushed the feature/refactor branch 2 times, most recently from 418f402 to a38b58c Compare June 17, 2021 14:11
@Nirmal4G Nirmal4G mentioned this pull request Jun 17, 2021
Merged
2 tasks
@Nirmal4G Nirmal4G marked this pull request as ready for review July 16, 2021 18:42
@Rosuavio Rosuavio mentioned this pull request Jul 20, 2021
Merged
4 tasks
Nirmal4G added 12 commits July 20, 2021 21:45
@Nirmal4G
Format and Update EditorConfig
4ec02d5 
- Update comments to be more to the point.
- Rearrange some lines in ascending order.
- Add more file types and their preferences.
- Rename missed files from previous renames.
@Nirmal4G
Refactor Core project files
3ad9062 
Organize Package tags
Update Package descriptions
Remove redundant MSBuild logic
Consolidate common MSBuild logic
@Nirmal4G
Refactor Test project files
8ed1caa 
Remove redundant MSBuild logic
Consolidate common MSBuild logic
Reorder code-blocks for better readability
Fix-up comments and new-lines across project files

Improve support for Visual Studio 2022:
Since VS IDE 2022 (17.0) is 64-bit only, replace all 32-bit specific MSBuild properties with generic ones.
@Nirmal4G
Optimize UITests Shared project's item includes
6303c67 
Remove redundant 'Exclude' as bin/obj doesn't exist in the Shared project.

We use None to include items to be visible under the Shared Project
in VS IDE. But having both None and Compile/Page includes for the
same files will impact Build and IDE perf.

So, Only include None when Compile/Page is already active.
@Nirmal4G
Manage NoWarn(s) from a single place
4c7e988 
Makes sure that global NoWarn(s) can be added/removed
without editing each and every project file.
@Nirmal4G
Update Solution to properly pack design tools
081e00b 
When the Controls project was exploded into several sub-projects,
they were not marked for build in the solution file, leading to the
missing of design tools assemblies in their respective control's packages.

This patch updates the solution file with build options similar to what
was already working, before the splitting of the Controls project.
@Nirmal4G
Consolidate package versions
9ddd405 
Use same versions of dependencies across projects
to ensure a consistent and predictable builds.

This also fixes a Warning (NU1603) in the UI Test projects.
@Nirmal4G
Update Assembly/Package Titles
b105331 
The package title doesn't follow a particular format and is not machine readable.
Thus, I propose the following format: "'Product' - 'Area' - 'Specifics' ('Source/Target')"
@Nirmal4G
Media Controls does not have a "DesignTools" project
1781e7f 
Remove targets that requires "DesignTools" check to let build succeed.
Revert this patch when adding the actual "DesignTools" project for Media Controls.
@Nirmal4G
Remove BOM (Byte Order Mark) from new files
a7f0b49 
VS IDE still adds this but its no longer needed!
@Nirmal4G
Rename Package Icon file only when packing
a2c2c92 
Every "official" .NET packages use 'Icon' as its file name.
Since, the icon file itself is public and older packages refer them by URL,
we only change the file name during packaging which would be in the package.
@Nirmal4G
Replace the term 'Basic' in Package Titles
e984265 
It was decided that the term 'Basic' is new and
has no established meaning in the software community.

So, Replacing the term 'Basic' with 'Common'. To disambiguate
multiple instances of 'Common', we use an already existing term
'Primitive' and we also append the Platform to which the package targets.
@michael-hawker
Copy link
Member

michael-hawker commented Jul 20, 2021

Apparently we have some merge conflicts now:

Directory.Build.props
Microsoft.Toolkit.Uwp.Notifications/Properties/Microsoft.Toolkit.Uwp.Notifications.rd.xml
Microsoft.Toolkit.Win32/README.md
UITests/UITests.Tests.MSTest/UITests.Tests.MSTest.csproj
UITests/UITests.Tests.Shared/UITests.Tests.Shared.projitems
UITests/UITests.Tests.TAEF/UITests.Tests.TAEF.csproj

It won't let me resolve them online here.

@ghost
Copy link

ghost commented Jul 20, 2021

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@ghost ghost removed the auto merge ⚡ label Jul 20, 2021
@Nirmal4G Nirmal4G force-pushed the feature/refactor branch 2 times, most recently from c3fcfb9 to e984265 Compare July 20, 2021 17:09
@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Jul 20, 2021

Ahh… The PRs #4090 and #4117 broke this. I've fixed the conflicts.

@michael-hawker michael-hawker merged commit 747497a into CommunityToolkit:main Jul 20, 2021
@Nirmal4G Nirmal4G deleted the feature/refactor branch July 20, 2021 18:35
@Nirmal4G Nirmal4G mentioned this pull request Sep 8, 2021
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azchohfi azchohfi azchohfi left review comments

@michael-hawker michael-hawker michael-hawker approved these changes

@Rosuavio Rosuavio Awaiting requested review from Rosuavio

+1 more reviewer

@jeromelaban jeromelaban jeromelaban left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

improvements ✨ needs attention 👋

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

Consolidate common build logic [Proposal] New Assembly/Package Title format

4 participants

@Nirmal4G @michael-hawker @azchohfi @jeromelaban