Skip to content

Remove unused private parameters in SPC - #63015

Merged
marek-safar merged 9 commits into
dotnet:mainfrom
marek-safar:ide0060
Feb 12, 2022
Merged

Remove unused private parameters in SPC#63015
marek-safar merged 9 commits into
dotnet:mainfrom
marek-safar:ide0060

Conversation

@marek-safar

@marek-safarmarek-safar commented Dec 20, 2021

Copy link
Copy Markdown
Contributor

and remove related dead code

@ghost

ghost commented Dec 20, 2021

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@marek-safar
marek-safarforce-pushed the ide0060 branch 2 times, most recently from 3c0f748 to a0c6c87CompareDecember 21, 2021 16:03
@runfoapprunfoappBot mentioned this pull request Dec 21, 2021
Closed
@marek-safar
Remove unused parameters in SPC
72fa691
and remove related dead code
@marek-safar
marek-safar marked this pull request as ready for review December 23, 2021 15:22
@ghost

ghost commented Dec 23, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

and remove related dead code

Author:marek-safar
Assignees:marek-safar
Labels:

area-System.Runtime

Milestone:-

jkotas
jkotas reviewed Dec 25, 2021
View reviewed changes
Comment threadsrc/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.cs Outdated
Comment threadsrc/libraries/System.Private.CoreLib/src/System/ByReference.cs Outdated
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.Unix.cs Outdated
Comment threadsrc/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs Outdated
@jkotas

jkotas commented Dec 28, 2021

Copy link
Copy Markdown
Member

Many instances of this stylecop warning were cleaned up in dotnet/coreclr#26104, but the enforcement was not enabled enforcement as part of that PR. cc @stephentoub

jkotas
jkotas reviewed Dec 29, 2021
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs
internal static bool CanSetMaxIOCompletionThreads(int ioCompletionThreads) => true;
#pragma warning restore IDE0060

[Conditional("unnecessary")]

@jkotasjkotasDec 29, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some of the similar cases are partial methods. Is there are reason why these are Conditional("unnecessary") and not partial methods?

@marek-safarmarek-safarDec 30, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

They are not private hence we cannot use a partial method without an implementation.

eerhardt
eerhardt reviewed Jan 3, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Unix.cs
Comment threadsrc/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs Outdated
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.cs Outdated
@stephentoub

stephentoub commented Jan 3, 2022

Copy link
Copy Markdown
Member

Many instances of this stylecop warning were cleaned up in dotnet/coreclr#26104, but the enforcement was not enabled enforcement as part of that PR. cc @stephentoub

Because there was a lot left to do across the repo :)

@stephentoub

stephentoub commented Jan 3, 2022•
edited
Loading

Copy link
Copy Markdown
Member

@marek-safar, are you planning to fix all occurrences of this across the repo and enable the rule?

Otherwise, I'm not a big fan of adding a lot of suppressions for a rule we explicitly disable.

@marek-safar

marek-safar commented Jan 3, 2022

Copy link
Copy Markdown
ContributorAuthor

are you planning to fix all occurrences of this across the repo and enable the rule?

I'd like in 2022 ;-)

@runfoapprunfoappBot mentioned this pull request Jan 3, 2022
Open
eerhardt
eerhardt reviewed Jan 4, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs Outdated
adamsitnik
adamsitnik reviewed Jan 28, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs Outdated
Comment threadsrc/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
Comment threadsrc/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs Outdated
Comment threadsrc/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs Outdated
eerhardt
eerhardt approved these changes Feb 11, 2022
View reviewed changes

@eerhardteerhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@marek-safar
marek-safar merged commit 47bcd1a into dotnet:mainFeb 12, 2022
@marek-safar
marek-safar deleted the ide0060 branch February 12, 2022 17:44
@ghostghost locked as resolved and limited conversation to collaborators Mar 14, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@adamsitnikadamsitnikadamsitnik left review comments
@jkotasjkotasjkotas left review comments
@eerhardteerhardteerhardt approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner
@stephentoubstephentoubAwaiting requested review from stephentoub

Assignees

@marek-safarmarek-safar

Labels

area-System.Runtime

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@marek-safar@jkotas@stephentoub@adamsitnik@eerhardt