Skip to content

Conversation

@Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented Dec 22, 2021

Part of #4779

Context

Make Common props, targets and tasks easier to read and understand.
Make Visual Studio's Property Page (XAML) Rules easier to read and understand.

Part of the larger refactoring that'll lead into #1686

Changes Made

  • Remove all trailing spaces
  • Fixup new lines where necessary
    • Add new lines between every block to make it clear.
    • Remove unnecessary new lines to reduce scrolling.
  • Adjust Indentation as per EditorConfig (2-space)

Notes

I'll also make sure not to mess up the git blame too much.
To make reviewing easier, I had split up the changes logically rather than a file.

The commits by themselves are logical changes. So, if possible, rebase merge them to preserve file history view in Git.

Also, please do note that these changes won't make much difference alone but with other formatting and refactors that follow this will make a world of difference in reading these files.

@Nirmal4G
Copy link
Contributor Author

As you can see, some source files have also been touched. They belong to a different set of patches but I have extracted whitespace changes early on.

@Nirmal4G
Copy link
Contributor Author

The PR #7165 removes the logic related to compatibility with MSBuild v4 and Dev11/Dev12. If that is approved, much of the changes here wouldn't apply. So, please wait until there's a verdict on that PR! And I'll rebase the PR accordingly.

Copy link
Contributor

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Nirmal4G Nirmal4G force-pushed the hotfix/core-sdk-prep/clean-up branch 2 times, most recently from 7dc9239 to 1d1c988 Compare January 9, 2022 10:40
@Nirmal4G Nirmal4G requested a review from Forgind January 9, 2022 10:53
@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Jan 9, 2022

All done. Updated in response to #7165!

@Nirmal4G Nirmal4G force-pushed the hotfix/core-sdk-prep/clean-up branch from 1d1c988 to e175fc8 Compare January 21, 2022 14:34
@Nirmal4G
Copy link
Contributor Author

Updated in response to #7169!

@Nirmal4G Nirmal4G force-pushed the hotfix/core-sdk-prep/clean-up branch from e175fc8 to 50b74ae Compare February 20, 2022 07:55
@benvillalobos
Copy link
Member

@Nirmal4G were these changes made via VS or some tool? The diff is massive, but if I can review each commit knowing it was done by automation it'd be easier to glance through instead of parsing every change.

Side note: this should merge as 1 commit

@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Apr 22, 2022

@benvillalobos Only 2nd commit (not all lines) were done by me, manually.

this should merge as 1 commit

You'll lose the clean diff, if not rebased. Anyway, it's just my suggestion.

About my PR tree format

I don't know what @microsoft's policy is but where I work, we have archive policy with OSS projects, where we take all the contributions up until major or minor releases and clean them up. After a few major versions, we take that archive's latest as base. And we'll do this over and over again.

Hence, having separated like this will make it easier for our archivists easier to clean up the commit tree. It'll also make it easier to blame and dig up historical commits easily and compare. Recently we also started using blame ignores to filter out the cosmetic commits.

Nirmal4G added 3 commits June 3, 2022 07:14
These are the files that would be touched by VS Editor later.
Separating the whitespace changes early on would help reviewers.
Remove EOF New Lines
Remove Unnecessary New Lines
Add New Lines between code/comment blocks

This is done separately to ensure that the diff for the actual changes would be as clean as possible.
Follow 2-space indent everywhere

For Multi-line comments, text starting between
comment tags should be 4-space and extra 2-space
for the text that follows from the previous line.

This is done separately to ensure that the diff for the actual changes would be as clean as possible.
@Nirmal4G Nirmal4G force-pushed the hotfix/core-sdk-prep/clean-up branch from 50b74ae to 5648271 Compare June 3, 2022 01:51
@Nirmal4G Nirmal4G requested a review from Forgind June 3, 2022 02:21
@benvillalobos benvillalobos added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jun 3, 2022
@benvillalobos benvillalobos merged commit 142158c into dotnet:main Jun 9, 2022
@Nirmal4G Nirmal4G deleted the hotfix/core-sdk-prep/clean-up branch June 9, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants