Update rule docs by sdwheeler · Pull Request #1711

Update rule docs by sdwheeler · Pull Request #1711 · PowerShell/PSScriptAnalyzer

PR Summary

Updated the README and Rules documentation.

Corrected grammar and formatting.
Added column to table in README to indicate which rules are enabled by default.
Renamed files to match the actual rule names as used by the end-user

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

Choose a reason for hiding this comment

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

Approved, mostly just formatting changes but did try to read every revised wording, I was looking for grammar and just basic high level things, not really if rules/docs were "correct" because I am unsure about every rule. I learned somethings even reading through :)

Choose a reason for hiding this comment

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

about halfway done reviewing, will take time to go through it thoroughly

Comment on lines +78 to +82

		The compatibility analysis compares a command used to both a target profile and a "union" profile
		(containing all commands available in any profile in the profile dir). If a command is not present
		in the union profile, it is assumed to be locally created and ignored. Otherwise, if a command is
		present in the union profile but not present in a target, it is deemed to be incompatible with that
		target.

Choose a reason for hiding this comment

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

This documentation was written with semantic line breaks in mind. While those aren't used in the docs repos, I think we wish to keep them here.

		If a ScriptBlock is intended to be run in a new RunSpace, variables inside it should use the
		`$using:` scope modifier, or be initialized within the ScriptBlock. This applies to:

		- `Invoke-Command`- Only with the ComputerName or Session parameter.

Choose a reason for hiding this comment

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

Here I'd suggest -ComputerName etc, in monospace

Choose a reason for hiding this comment

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

Bold is the style when parameter names are used in a paragraph. Code style is used when the parameter is used in a code context.

		## Description

		For a file encoded with a format other than ASCII, ensure Byte Order Mark (BOM) is present to ensure that any application consuming this file can interpret it correctly.
		For a file encoded with a format other than ASCII, ensure Byte Order Mark (BOM) is present to ensure

Choose a reason for hiding this comment

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

Is this still true or do we expect UTF8 (no BOM) now?

Choose a reason for hiding this comment

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

The rule exists. I guess the question is whether it is or should be applied?

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

(As it happens, that doc page itself has encoding issues around HTML escaping)

Choose a reason for hiding this comment

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

Should we add the information about WinPS to the rule doc?

Choose a reason for hiding this comment

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

@rjmholt FYI - just fixed the html encoding problem in the article. It will be live this afternoon.

Co-authored-by: Robert Holt <rjmholt@gmail.com>
Co-authored-by: Steve Lee <slee@microsoft.com>

@SteveL-MSFT this is set to auto-merge, so when you refresh your review to an approval it will merge