Several suggestions to make it better #162
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As Windows is not the only OS PowerShell can run on, having 'Windows' in the name is not very practical anymore. That's why, starting with version 6, Microsoft decided to change 'Windows PowerShell' to 'PowerShell'. The text now follows this.
Two times 'Powershell' was written in this text instead of 'PowerShell', which makes them inconsistent with the rest of the text where it was properly written as 'PowerShell'.
The proper name of '.NET' is '.NET', all capitals, not '.Net'.
Change 're-use' to 'reuse' and 're-used' to 'reused', as these words are written as such in both Merriam-Webster and the Cambridge Dictionary. Furthermore, in another location in the text the word was already written as 'reuse' and 'reusable', so it was also inconsistent.
In the explanation about controller scripts, it read 'A script is not intended to be reusable', which made no sense to me, so I changed it to 'A controller script is not intended to be reusable'.
In the example paragraph under TOOL-01, it is clear a controller script is being explained. In the paragraph following that, a further remark is made about controller scripts, but it read 'Controllers, on the other hand, (...)', which seems to imply the paragraph before that was talking about a tool script, which wasn't the case (it was talking about a controller script helped by several tool scripts, so tool scripts were also mentioned, but the paragraph ends with two sentences about the controller script, so 'on the other hand' in the next paragraph is incorrect and confusing IMHO).
Some paragraphs were separated by one empty line, some by two. The second empty line at several places has been deleted.
At two locations, a dot was missing at the end of the sentence. This has been corrected.
In the text it was written '(..) in regards parameter naming'. This is incorrect, according to both Merriam-Webster and the Cambridge Dictionary: it is 'in regard to'. This has been corrected.
At one location 'Pipeline' was written and everywhere else 'pipeline', which is inconsistent, so 'Pipeline' has been changed to 'pipeline'.
Changed some typos like 'a interactions' and the use of 'i.e.' (misspelled as 'ie'), which means 'that is', while 'e.g.' seemed to be the correct abbreviation in that context, which means 'for example'.
- Add space between 'foreach' and the following parenthesis. - Change the faulty 'a la' to the correct 'à la'. - Write '.NET' and '.NET Framework' consistently the same throughout.
Delete extra line.
- Change '.Net' to '.NET' to keep it consistent. - Change incorrect usage of spaces within example code (space after opening parenthesis and space before closing parenthesis). - Change several inconsistencies in example code: - Change '[System.Runtime.InteropServices.marshal]' to '[System.Runtime.InteropServices.Marshal]'. - Remove semicolon at the end of the lines. - Remove an unnecessary 'return' keyword. - Remove extra line at the end.
- Change '.Net' to '.NET' to keep it consistent. - Remove several spurious blank lines. - Change 'PowerShell Classes' to 'PowerShell classes'. - Change "'s" to "or". - Change 'makes sense more sense' to 'makes more sense'. - Move 'param()' in '[CmdletBinding()]param()' to the next line. - Change 'comment based help' to 'comment-based help'. - Change 'a about_ModuleName' to 'an about_ModuleName'. - Change 'that pases parameters positionally' to 'that passes parameters positionally'. - Change 'PsCmdlet.ThrowTerminatingError' to '$PSCmdlet.ThrowTerminatingError'. - Change 'PsCmdlet.WriteError' to '$PSCmdlet.WriteError'. - Change inconsistencies with the styling guide/conventions in the example code for function ThrowError: - Change typo 'errorrecord' to 'error record'. - Move brace after 'function ThrowError' to the same line. - Move parenthesis after 'param' to the same line. - Change 'parameter' to 'Parameter'. - Change '$exception = New-Object $ExceptionName $ExceptionMessage;' to '$exception = New-Object -TypeName $ExceptionName -ArgumentList $ExceptionMessage' (so using named parameters instead of positional parameters and removing the unnecessary semicolon at the end). - Change 'Discuss: when is this critical (-whatif) and optional (-confirm_' to 'Discuss: when is this critical (-WhatIf) and optional (-Confirm)'. - Change 'Discuss: when should you call PSCmdlet.ShouldProcess vs PSCmdlet.ShouldContinue (-Force)' to 'Discuss: when should you call $PSCmdlet.ShouldProcess vs $PSCmdlet.ShouldContinue (-Force)'. - Change more inconsistencies in example code: - Remove unnecessary semicolons at the end of the line. - Add space after 'foreach'. - Remove inconsistent spacing. - Add space after 'if'. - Change more small typos. P.S.: Make '$errorRecord = New-Object System.Management.Automation.ErrorRecord $exception, $ErrorId, $ErrorCategory, $ExceptionObject' code that actually works! This code will generate the error **New-Object: Cannot bind argument to parameter 'TypeName' because it is null.**!
- Change "what it's expected or allowed values are" to "what its expected or allowed values are" as clearly "it's" cannot mean 'it is' or 'it has' in this context and is thus incorrect. - Change 'You should support --whatif' to 'You should support -WhatIf' as that's the correct casing. Also, the double dash has been changed to a single dash because this is PowerShell, not sh, bash, zsh or something like that. - Remove the unnecessary semicolon at the end of the line in the example code. - Change some small typos and remove more than one blank line in several places. P.S. Consider using single quotes for strings with constant values instead of double quotes! Using PSScriptAnalyzer will catch these! By the way, consider recommending using PSScriptAnalyzer, because using that will generally improve one's code significantly!
gaelcolas
approved these changes
Apr 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Several suggestions to make it better, to fix typos, fix inconsistencies, etc.