-
Notifications
You must be signed in to change notification settings - Fork 394
Fix AvoidInternalURLs throw warnings at SDDL #46
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.
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
Conversation
Take bug fixes to Master
Add Tests for UseShouldProcessForStateChangingFunctions
Remove check for "Get-'" Get does not modify system Status
Conflicts: Rules/UseShouldProcessForStateChangingFunctions.cs
[4/17] Take this week's bug fixes to Master branch
if (count !=3 ) | ||
{ | ||
isInternalURL = true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an SDDL can actually have 4 ":"
This is because they can have both DACL and SACL components (SACL is optional, I think)
In the test case, the string only has the DACL component.
https://msdn.microsoft.com/en-us/library/windows/desktop/aa379570(v=vs.85).aspx
Thanks Quoc. I've changed the count to check both 3 and 4. |
@@ -3,7 +3,7 @@ | |||
|
|||
##Description | |||
|
|||
Functions that have verbs like New, Start, Stop, Set, Reset that change system state should support 'ShouldProcess' | |||
Functions that have verbs like New, Start, Stop, Set, Reset and Restart that change system state should support 'ShouldProcess' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is file in the current change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I committed the changes earlier to BugFixes by accident so I reverted the changes. This file may be reverted as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok thanks.
looks good. |
Looks good to me too. |
Fix AvoidInternalURLs throw warnings at SDDL
Thanks! |
No description provided.