-
Notifications
You must be signed in to change notification settings - Fork 398
Added an about help topic file and fixed a typo in rule description #275
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
…iCmdlet rule description.
Hi @rkeithhill, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
@rkeithhill, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
passing the rule and eliminating a "potential" issue with changing script | ||
and possibly introducing a new problem. In the end, for existing scripts, | ||
it is usually best to have evaluated the rule violations that you deem the | ||
most valuable to address. |
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.
We can add a paragraph about "Suppression" functionalities here?
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.
Sure, I did cover rule suppression a bit in lines 67 thru 86 but it is probably worth at least adding a mention to that section here. Or we can re-iterate that information as well. I'm open. :-) Did you want me to do that? I'm also fine if you want to edit/change anything in this help topic.
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.
It would be great if you can just add a line or two here to re-iterate :-) Thanks!
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.
Done. See the commit 8c244ce
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.
After looking at the comments on @splatteredbits PR and the sample that is referenced. I wonder if rule suppression should have its own help topic about_PSScriptAnalyzer_RuleSuppression.help.txt? One that can show the code samples at https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Tests/Engine/RuleSuppression.ps1 Thoughts?
…ING SCRIPT ANALYZER sections.
@rkeithhill . Thank you for working on this. @yutingc. Thanks for reviewing the changes. Changes look good. We can merge the PR. |
We have another PR for rule suppression documentation. We should consider merging these into about_ help topic |
Yeah looks good to me. We can merge the rule suppression into this later when it is done. |
Added an about help topic file and fixed a typo in rule description
The typo was in the PSAvoidUsing WmiCmdlet rule description. Sorry if this isn't targeting the correct branch. I'm a long time TFS user but a Git noob.