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 — UseApprovedVerbs.md: Improving Documentation by Banner-Keith · Pull Request #1294 · PowerShell/PSScriptAnalyzer

Conversation

@Banner-Keith

@Banner-Keith Keith Banner (Banner-Keith) commented Jul 22, 2019

Copy link
Copy Markdown
Contributor

PR Summary

I have come to this documentation a few times in the past. Get-Verb is very helpful, but the additional insight, and examples of unapproved verbs in the Microsoft docs are very helpful and make it easier to decide on a suitable replacement for an unapproved verb. I think it would be helpful to have a link to that documentation here.

The contribution guidelines say "Submit your own fixes or features as a pull request but please discuss it beforehand in an issue if the change is substantial." This change is not substantial and therefore I did not open an issue first.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Banner-Keith
Update UseApprovedVerbs.md
57448f9 
I have come to this documentation a few times in the past. Get-Verb is very helpful, but the additional insight, and examples of unapproved verbs in the Microsoft docs are very helpful and make it easier to decide on a suitable replacement for an unapproved verb. I think it would be helpful to have a link to that documentation here.
bergmeister
Christoph Bergmeister (bergmeister) approved these changes Jul 23, 2019
View reviewed changes
bergmeister
Christoph Bergmeister (bergmeister) reviewed Jul 23, 2019
View reviewed changes
Comment thread RuleDocumentation/UseApprovedVerbs.md Outdated

Approved verbs can be found by running the command `Get-Verb`.

Additional documentation on approved verbs can be found in the microsoft docs page [Approved Verbs for PowerShell Commands](https://docs.microsoft.com/en-us/powershell/developer/cmdlet/approved-verbs-for-windows-powershell-commands). If you find the verb you are using is unapproved, try searching the page for the approved equivalent. For example, if you search in the documentation for `Read`, `Open`, or `Search` you will find that the approved verb for those situations is `Get`.

@bergmeister Christoph Bergmeister (bergmeister) Jul 23, 2019

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. Looks good to me. Just a minor suggestion to take out the \en-us part of the link so that the page gets displayed in the user's locale.

Suggested change
Additional documentation on approved verbs can be found in the microsoft docs page [Approved Verbs for PowerShell Commands](https://docs.microsoft.com/en-us/powershell/developer/cmdlet/approved-verbs-for-windows-powershell-commands). If you find the verb you are using is unapproved, try searching the page for the approved equivalent. For example, if you search in the documentation for `Read`, `Open`, or `Search` you will find that the approved verb for those situations is `Get`.
Additional documentation on approved verbs can be found in the microsoft docs page [Approved Verbs for PowerShell Commands](https://docs.microsoft.com/en-us/powershell/developer/cmdlet/approved-verbs-for-windows-powershell-commands). If you find the verb you are using is unapproved, try searching the page for the approved equivalent. For example, if you search in the documentation for `Read`, `Open`, or `Search` you will find that the approved verb for those situations is `Get`.

Sorry, something went wrong.

Uh oh!

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

@Banner-Keith Keith Banner (Banner-Keith) Jul 23, 2019

Copy link
Copy Markdown
Contributor Author

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 the suggestion. I have removed that. Do I need to squash both changes into one commit before the pull request is completed?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bergmeister reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@bergmeister Christoph Bergmeister (bergmeister) Jul 23, 2019 •
edited
Loading

Uh oh!

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

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.

You're welcome. No need for squashing, when the PR gets merged all commits will get squashed automatically by GitHub and the final commit message will be the PR title and the description. I will assign a 2nd reviewer to as a best practice and we'll then take care of merging it in. Thanks :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Banner-Keith and rjmholt reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@Banner-Keith
Removed en-us from link
f36a892
@bergmeister Christoph Bergmeister (bergmeister) assigned Rob Holt (rjmholt) Jul 23, 2019
rjmholt
Rob Holt (rjmholt) reviewed Jul 23, 2019
View reviewed changes
Comment thread RuleDocumentation/UseApprovedVerbs.md Outdated

Approved verbs can be found by running the command `Get-Verb`.

Additional documentation on approved verbs can be found in the microsoft docs page [Approved Verbs for PowerShell Commands](https://docs.microsoft.com/powershell/developer/cmdlet/approved-verbs-for-windows-powershell-commands). If you find the verb you are using is unapproved, try searching the page for the approved equivalent. For example, if you search in the documentation for `Read`, `Open`, or `Search` you will find that the approved verb for those situations is `Get`.

@rjmholt Rob Holt (rjmholt) Jul 23, 2019

Copy link
Copy Markdown
Contributor

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.

Ideally this could be broken into semantic lines.

Sorry, something went wrong.

Uh oh!

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

@rjmholt Rob Holt (rjmholt) Jul 23, 2019

Copy link
Copy Markdown
Contributor

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.

The second sentence is backwards I think; it reads like:

If you are using an unapproved verb, try searching for its approved equivalent (which you're trying to find)

It might be worded better as:

Some unapproved verbs are documented on the approved verbs page and point to approved alternatives; try searching for the verb you used to find its approved form. For example, searching for Search or Open will lead you to Get.

Sorry, something went wrong.

Uh oh!

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

@Banner-Keith Keith Banner (Banner-Keith) Jul 23, 2019

Copy link
Copy Markdown
Contributor Author

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.

Good point. I'll update that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Keith Banner Keith Banner
Clarified text. Broke paragraph into semantic lines.
e92552a
rjmholt
Rob Holt (rjmholt) approved these changes Jul 23, 2019
View reviewed changes
@rjmholt

Rob Holt (rjmholt) commented Jul 23, 2019

Copy link
Copy Markdown
Contributor

Thanks for your contribution Keith Banner (@Banner-Keith)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rjmholt
Rob Holt (rjmholt) merged commit 17e9677 into PowerShell:master Jul 23, 2019
@Banner-Keith

Keith Banner (Banner-Keith) commented Aug 28, 2019

Copy link
Copy Markdown
Contributor Author

Rob Holt (@rjmholt) I noticed that the Show documentation hint in the PowerShell extension points at the development branch. That might be because I'm using the preview. Anyway. This change is not in the development branch, just in master. Could we merge it there too?
All reactions

Sorry, something went wrong.

Uh oh!

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

Christoph Bergmeister (bergmeister) commented Aug 28, 2019

Copy link
Copy Markdown
Collaborator

Keith Banner (@Banner-Keith) After the release of 1.18.1 we decided to abandon this model of using 2 branches and started using master only. The development branch is only there for legacy purposes and will not be updated any more. The next version of the VS-Code PowerShell extension will start pointing to master, this change was already merged: PowerShell/vscode-powershell#2037
PSScriptAnalyzer 1.18.2 will soon release in the next days and if everything works out well then after that the VS-Code extension will release as well a bit later.
We could merge the changes made in master back to development though if you need them now and don't want to wait?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Banner-Keith

Keith Banner (Banner-Keith) commented Aug 29, 2019

Copy link
Copy Markdown
Contributor Author

Christoph Bergmeister (@bergmeister) Thanks for the reply.
It sounds like there is a good solution going forward. I'm okay with waiting. Thanks for explaining how that will work.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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
+1 more reviewer
@rjmholt Rob Holt (rjmholt) rjmholt approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@rjmholt Rob Holt (rjmholt)

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.

3 participants

@Banner-Keith @rjmholt @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.