Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Repo clean-up for upcoming changes #4796

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Repo clean-up for upcoming changes #4796

wants to merge 10 commits into from

Conversation

Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented Oct 28, 2022 •
edited
Loading

Changes

  • Remove BOM from new files.
  • Remove redundant xml tags and attributes.
  • Update Git ignore and attributes.
  • Ensure to preserve encoding of UTF-16LE files.
  • Update Toolkit Icons and use Icon.png name instead.
  • Rename solution, scripts and NuGet config files.
  • Remove leftover files from previous refactors.
  • Format Notifications package ReadMe.
  • Remove redundant code from project files.

PR Type

What kind of change does this PR introduce?

  • Code style update (formatting only)
  • Refactoring (no functional changes, no API changes)
  • Build or CI related changes (non-breaking)

PR Checklist

Please check if your PR fulfills the following requirements:

  • Created a separate branch (other than main/master) in your fork
  • Based off latest main branch of toolkit
  • Pull Request doesn't include merge commits
  • Header has been added to all new source files
  • Contains NO breaking changes
  • Code follows all style conventions

Other information

  • Please Rebase merge if possible.
  • Please update the default message option to Default to pull request title and description in the Repository Pull Request settings to have a better commit message instead of Merge pull request #xxxx from repo/branch generic message.

@Nirmal4G
Remove BOM from new files
4659c8c 
Renormalize new lines and remove BOM from new files
@Nirmal4G
Remove redundant xml tags and attributes
02224f0 
- Remove `<?xml?>` file-type tag from all files.
- Remove `xmlns` and `ToolsVersion` attributes from project files.
@Nirmal4G
Update Git ignore and attributes
e489596 
- Add default Git attributes.
- Update Git ignores to latest.
@Nirmal4G
Ensure we preserve the encoding of UTF-16LE files
3306af2 
- Use `*.<encoding>.*` (like `*.UTF-16LE.*`) generalized naming scheme to mark differently encoded files.
@Nirmal4G
Update Icons and use Icon.png name instead
8bcaab9 
- Add `Icon.png` apart from `nuget.png`
  - This is used as a public reference point for NuGet icon URL. So, keep the original file.
  - If there's a need to update the icon, only update the new one `Icon.png` and keep the old one intact.
@Nirmal4G
Rename solution, scripts and NuGet config files
ce7a511 
- Rename solution file to `CommunityToolkit.Windows.sln`
  - The name starting with `Windows` doesn't stand-out in the file list because of the sort order of the `W` char.
  - Spaces are a main issue when doing automation (_like using `*.sln` in build scripts and in URLs it adds `%20`_).

- Fix casing for NuGet config files.

- Rename Smoke Tests PowerShell scripts.
@Nirmal4G
Remove leftover files from previous refactors
8cd5355 
- Remove empty leftover build files which are not being used anyway.
@Nirmal4G
Format Notifications package ReadMe
f5f2a9d 
- Fix reported MarkdownLint warnings
- Format tables, use code blocks and adjust spacing.
@Nirmal4G
Remove redundant code from project files
6b563f8 
- Remove un-necessary attributes.
- Remove un-used `BinDir` item metadata.
@Nirmal4G
Update comments and fix mistakes
fbb24ee 
- Fix message text.
- Fix Include logic.
- Fix spelling mistakes.
- Fix leading whitespace.
- Fix casing in code and comments.
@ghost
Copy link

ghost commented Oct 28, 2022

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 michael-hawker and azchohfi October 28, 2022 06:55
@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Oct 28, 2022 •
edited
Loading

Most of these changes match the changes from the .NET Toolkit repo. Some are specific to this Repo. Extracted from #4234 as it's becoming too large.

@Nirmal4G Nirmal4G force-pushed the repo/clean-up branch 3 times, most recently from 896e417 to 8297f88 Compare October 28, 2022 13:20
@Nirmal4G Nirmal4G marked this pull request as draft October 28, 2022 19:15
@Nirmal4G Nirmal4G force-pushed the repo/clean-up branch 2 times, most recently from 2f80273 to fbb24ee Compare November 24, 2022 18:55
@michael-hawker
Copy link
Member

Hi @Nirmal4G, we're working on re-doing all the infrastructure of the Toolkit based on the Labs infrastructure (as called out in our 8.0 plans in #4488).

If you'd like to help us out, the labs repo is here. Most of the infrastructure is in the common folder. If you do see things you think could be improved upon, could you please open an issue there first detailing the changes you'd suggest before opening a PR? That way we can discuss and coordinate with others working on the repo, as there's a lot of shared infrastructure between the templates and the experiments themselves.

Thanks!

@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Dec 4, 2022

@michael-hawker

If you'd like to help us out, the labs repo is here. Most of the infrastructure is in the common folder.

Yes, I have looked into it and I do think there's a lot that can be improved but I need to validate my solutions first before committing anything.

we're working on re-doing all the infrastructure of the Toolkit based on the Labs infrastructure.

Then, have you already began work on refactoring this repo on top of the new build infra?

@michael-hawker
Copy link
Member

Then, have you already began work on refactoring this repo on top of the new build infra?

@Nirmal4G we're working on abstracting a few things in the Labs repo still so that we can basically just make the common and template folders submodules.

Looking at what we're doing in Labs is the best path forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michael-hawker michael-hawker Awaiting requested review from michael-hawker

@azchohfi azchohfi Awaiting requested review from azchohfi

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Nirmal4G @michael-hawker