GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix typo in example by Farwaykorse · Pull Request #1379 · PowerShell/PSScriptAnalyzer

Conversation

@Farwaykorse

@Farwaykorse Farwaykorse commented Dec 3, 2019

Copy link
Copy Markdown
Contributor

Correct the script name to match the name in the example.

Removed the when using. It belonged with the when using Invoke-ScriptAnalyzer used in the previous example and repeating that seems unnecessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Farwaykorse
Fix typo in example in README.md
dba3687
bergmeister
Christoph Bergmeister (bergmeister) reviewed Dec 3, 2019
View reviewed changes

@bergmeister Christoph Bergmeister (bergmeister) left a comment

Copy link
Copy Markdown
Collaborator

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.

Thanks for making it consistent.
I am happy with the proposed change but I think we could improve it further by making 2 more changes:

  • Use the full parameter name, which is -Settings in both examples
  • Replace PSScriptAnalyzerSettings.psd1 in the 2 examples with $PSScriptAnalyzerSettingsPath. This is because using -Settings PSScriptAnalyzerSettings.psd1 is redundant due to the implicit settings feature.

Let me know what you think.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Farwaykorse
Use full parameter name Settings in README.md
d72779e
@Farwaykorse

Farwaykorse commented Dec 3, 2019

Copy link
Copy Markdown
Contributor Author

I agree with the use of -Settings.

I wouldn't use the variable, it just adds confusion over the meaning of the variable.
e.g. When you wrote it, my first assumption was that it was a variable defined by PSScriptAnalyzer.

The current use of PSScriptAnalyzerSettings.psd1 as a filename shows the examples as they apply to both the implicit use and by user supplied path.
bergmeister reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bergmeister

Christoph Bergmeister (bergmeister) commented Dec 3, 2019

Copy link
Copy Markdown
Collaborator

Ok, happy with that as well, it is definitely an improvement. Thanks 😀
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bergmeister
Christoph Bergmeister (bergmeister) approved these changes Dec 3, 2019
View reviewed changes
@bergmeister
Christoph Bergmeister (bergmeister) merged commit 20c4611 into PowerShell:master Dec 3, 2019
@Farwaykorse
Farwaykorse deleted the patch-2 branch December 3, 2019 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bergmeister Christoph Bergmeister (bergmeister) bergmeister approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@Farwaykorse @bergmeister
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.