8000 Remove redundant local assignment in AclCommands by xtqqczze · Pull Request #14358 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@xtqqczze
Copy link
Contributor
@xtqqczze xtqqczze commented Dec 9, 2020

Follow-up to #14341.

@rjmholt
Copy link
Collaborator
rjmholt commented Dec 9, 2020

@PoshChan please remind me in 1 day

@rjmholt rjmholt added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 9, 2020
// Add CAP names and IDs to a string array.
string[] policies = new string[capCount];
NativeMethods.CENTRAL_ACCESS_POLICY cap = new NativeMethods.CENTRAL_ACCESS_POLICY();
NativeMethods.CENTRAL_ACCESS_POLICY cap;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be removed here and instead be defined in the for loop body scope, where it is only used (similar to pCapId)/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this code is inside of an if !CORECLR do you still want the change made?


// Find the supplied string among available CAP names, use the corresponding CAPID.
NativeMethods.CENTRAL_ACCESS_POLICY cap = new NativeMethods.CENTRAL_ACCESS_POLICY();
NativeMethods.CENTRAL_ACCESS_POLICY cap;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be removed here and instead be defined in the for loop body scope, where it is only used.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 9, 2020
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 9, 2020
@xtqqczze xtqqczze changed the title Remove useless assignment to locals in AclCommands Remove redundant local assignment in AclCommands Dec 9, 2020
Copy link
Contributor
@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

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

LGTM

@xtqqczze
Copy link
Contributor Author
xtqqczze commented Dec 9, 2020

@PaulHigin I've pushed some more changes on the same theme.

@rjmholt
Copy link
Collaborator
rjmholt commented Dec 9, 2020

@PoshChan please remind me in 20 hours

@PoshChan
Copy link
Collaborator

@rjmholt, this is the reminder you requested 20 hours ago

@rjmholt rjmholt merged commit a8213b5 into PowerShell:master Dec 10, 2020
@PoshChan
Copy link
Collaborator

@rjmholt, this is the reminder you requested 1 day ago

@ghost
Copy link
ghost commented Dec 15, 2020

🎉v7.2.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

@xtqqczze xtqqczze deleted the sacl branch July 3, 2021 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0