Skip to content

add new snippet: new Azure Resource Group #1549

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

Merged
merged 3 commits into from
Oct 2, 2018

Conversation

vmsilvamolina
Copy link
Contributor

@vmsilvamolina vmsilvamolina commented Sep 27, 2018

PR Summary

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • 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


```json
"New Azure Resource Group": {
"prefix": "rg",
Copy link
Contributor

Choose a reason for hiding this comment

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

If we follow the example set by the Send-MailMessage snippet (and other snippets that ship in the extension) perhaps the prefix should be ex-New-AzureRmResourceGroup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we follow the example set by the Send-MailMessage snippet (and other snippets that ship in the extension) perhaps the prefix should be ex-New-AzureRmResourceGroup?

@rkeithhill Yes! You´re correct... I modified the snippet name with your suggestion. Thanks for the comment.

Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

LGTM

@rjmholt
Copy link
Contributor

rjmholt commented Sep 28, 2018

I'll let @tylerl0706 sign off on this one :)

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

Content looks great! We just need it to be in the right order 👍 fix that and this is good to go!

@vmsilvamolina
Copy link
Contributor Author

@tylerl0706 @rkeithhill I have another snippets to add: Should I modify this PR or I need to create a new?

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@TylerLeonhardt
Copy link
Member

@vmsilvamolina lets do a new one and I'll merge this one in now 😄

@TylerLeonhardt TylerLeonhardt merged commit 7350d23 into PowerShell:master Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants