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

Clean-up white-space in some of the source files #6565

Merged
merged 4 commits into from
Jun 18, 2021
Merged

Clean-up white-space in some of the source files #6565

merged 4 commits into from
Jun 18, 2021

Conversation

Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented Jun 15, 2021 •
edited
Loading

Part of #4779

Context

Clean-up white-space in some of the source files (files that could be a part of my later PRs).

Changes Made

  1. Remove all trailing spaces.
  2. Fixup new-line(s) where necessary
    • Add New Line(s) between every block to make it clear.
    • Remove unnecessary New Line(s) to reduce scrolling.
  3. Remove unnecessary file (src/Framework/Event args classes.cd)

Notes

These are the files that would be touched by later PRs.
Separating the white-space changes early on would help reviewers.

rainersigwald
rainersigwald reviewed Jun 15, 2021
View reviewed changes
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Please don't rename src/Framework/Event args classes.cd. We should probably just delete it (in a separate PR).

Looking good overall!

eng/common/build.ps1 Outdated Show resolved Hide resolved
src/Deprecated/Engine/Shared/FrameworkLocationHelper.cs Outdated Show resolved Hide resolved
src/Shared/UnitTests/FileMatcher_Tests.cs Outdated Show resolved Hide resolved
@Nirmal4G
Copy link
Contributor Author

src/Framework/Event args classes.cd is just one file. Do we need a separate PR for that? Are there any other files that are not needed anymore?

@dotnet-maestro @rainersigwald
[vs16.11] Update dependencies from dotnet/arcade (#6567)
5de4459 
Update dependencies from https://github.com/dotnet/arcade build 20210615.2

Microsoft.DotNet.Arcade.Sdk
 From Version 5.0.0-beta.21226.1 -> To Version 5.0.0-beta.21315.2

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Rainer Sigwald <[email protected]>
rainersigwald
rainersigwald requested changes Jun 16, 2021
View reviewed changes
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

You can leave the deletion of the .cd file in this PR if you rebase it into its own commit, please.

scripts/Enumerate-MSBuild.ps1 Outdated Show resolved Hide resolved
@Nirmal4G
Remove unnecessary files
d3de980 
Missed files from previous removals
@Nirmal4G
Clean-up whitespace everywhere else
f30fcce 
except in Common props, targets and tasks.

1. Remove all trailing spaces.

2. Fixup NewLine(s) where necessary
    - Add New Line(s) between every block to make it clear.
    - Remove unnecessary New Line(s) to reduce scrolling.

These are the files that would be touched by VS Editor later.
Separating the whitespace changes early on would help reviewers.
@rainersigwald rainersigwald changed the base branch from vs16.11 to main June 16, 2021 17:34
@rainersigwald
Copy link
Member

Looks good! I don't think this meets the bar for 16.11, so I retargeted to main. Which naturally caused some merge conflicts because #6547 hasn't landed yet. But "fortunately" we can't check in to main right now because of problems on the Visual Studio side. I don't think you'll need to do anything, and we should be good to go in a few days. Thanks!

(leaving "change requested" so I remember to check on this but I actually approve)

@KirillOsenkov
Copy link
Member

So, could you say that this PR... nirmalizes whitespace?

@Nirmal4G
Copy link
Contributor Author

@KirillOsenkov There's lot more to nirmalize!! 😜😂

rainersigwald
rainersigwald approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Looks unblocked to me. Not sure why GitHub shows the diff as including release notes and stuff; those commits don't seem to be different. I'm going to close/reopen the PR and see if that helps it.

@rainersigwald rainersigwald reopened this Jun 18, 2021
Forgind
Forgind approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@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.

The extra commits you noticed are still there, but that doesn't bother me too much. Fine with me if rainersigwald is fine with it.

Thanks Nirmal4G!

@rainersigwald rainersigwald merged commit dec13b1 into dotnet:main Jun 18, 2021
@rainersigwald
Copy link
Member

They weren't really there; I manually merged to be extra sure of that :)

Thanks, @Nirmal4G!

@Nirmal4G Nirmal4G deleted the hotfix/core-sdk-prep/cleanup-whitespace branch June 18, 2021 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KirillOsenkov KirillOsenkov KirillOsenkov approved these changes

@rainersigwald rainersigwald rainersigwald approved these changes

@Forgind Forgind Forgind approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@Nirmal4G @rainersigwald @KirillOsenkov @Forgind