-
Notifications
You must be signed in to change notification settings - Fork 60.3k
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
PowerShell Quickstart #284
Conversation
guessing version support, based on python
Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
Thanks @potatoqualitee 💝 We'll get this over to the @github/docs-content-ecosystem to review! |
Note that I recommended a PSScriptAnalyzer Action in the comments. The Pester project just reached out and suggested we recommend their gorgeous action as well, I'd love to! https://github.com/marketplace/actions/publish-unit-test-results Is this something I can throw into the comments or expand upon? I didn't know about their action before and now I'm going to update my own projects. |
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.
@potatoqualitee wow!!! This is A-MAZ-ING! 👏 👏 👏
Thank you so much for putting this together and adding the additional PRs for the caching example and starter template. I will preface this review with the fact that I have NEVER used PowerShell, and I'll reach out to see if someone with more PowerShell experience can also take a look.
You did an excellent job emulating our other language guides when putting this together. Thank you for that! I had a few high-level questions about how the template relates to other sections of the guide. With some of those questions answered, I can suggest some more specific changes.
Thanks again for putting together such a thorough guide. This will be great for the GitHub community! ❤️
|
||
steps: | ||
- uses: actions/checkout@v1 | ||
- name: Perform a Pester test from the command-line to ensure expected results |
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.
Can you describe a bit more about what this step does? Is it part of a typical CI workflow?
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.
Pester testing is indeed part of a typical PowerShell CI workflow. The test is useless but provides a valid and simplified example that will work on any machine since Get-ChildItem
is universally available.
Generally, we'll test from a file, and not right from the command line. I've done it at the command line before, then moved it to file.
What I like about this example is that it shows PowerShell being used at the command line w/o being executed from a file. I do execute a lot of PS right from run:
- just not as much for Pester testing. I hope that's clear 🤞 I'm open to removing it. If you'd like to keep it, perhaps I can do some sort of ping test instead using Test-NetConnection
.
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 all your great work here, @potatoqualitee ⚡ I also like having a command line example that demonstrates Pester. What do you think of an example that demonstrates how to use test-path
from both the command line and the Unit.Tests.ps1
file? With that approach, the reader can understand that there are two different ways of doing the same thing, but will appreciate that the Tests file is more scalable. I could also add in these updates if you like.
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.
@martin389 sounds good. I had something similar in the original but the test was so silly, I ended up removing it. I'd love a better example. please add the updates 🚀
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.
Okay, great 🎉 I've added a commit with the changes 👍
|
||
### Starting with the PowerShell workflow template | ||
|
||
{% data variables.product.prodname_dotcom %} provides a PowerShell workflow template that should work for most PowerShell projects. This guide includes examples that you can use to customize the template. For more information, see the [PowerShell workflow template](https://github.com/actions/starter-workflows/blob/main/ci/powershell.yml). |
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 noted that this guide is dependent on getting the new Powershell workflow template you created merged. I wonder if you could decouple that dependency a bit. For example, you could remove the references to the starter workflow and just use the workflow you provide as an example. Then when the starter workflow you created is merged, this guide could be updated to reference it!
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.
Is it me that updates the guide? I have a bad memory and worry this 💎 will forgotten.
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.
Let's visit this right before we get this guide merged. If the starter template you created is still waiting for a review, you or a docs team member can update the guide to remove the reference to the starter workflow as a last step.
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.
I agree that it makes sense to decouple the dependency for now. @potatoqualitee - would you mind if I revised this example into a standalone workflow?
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.
@martin389 please do 🙏
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.
I've added a commit for this now 😄
Co-authored-by: Rachael Sewell <rachmari@github.com>
Co-authored-by: Rachael Sewell <rachmari@github.com>
- Added some small edits. - Converted example into a standalone. - Switched example to test-path. - Added explanations for powershell actions. - Added screenshot of example test failure.
Hi @potatoqualitee - I've added a commit with these changes:
Would these work for you? 🙇♂️ |
Related issue: #283
Why:
As a PowerShell developer, I find GitHub Actions extremely useful and very straightforward. PowerShell is built into each of the runner images, and is supported out of the box. It has a strong CI/CD presence and it makes a ton of sense to include PowerShell in the quickstart.
Note that I used the
Building and testing Python
as the template for language and format for consistency.Also, two other PRs must be merged for this PR to be complete:
actions/cache#431
actions/starter-workflows#661
What's being changed:
Added an entire PowerShell / Pester quickstart
Check off the following:
Not sure what
reviewed my changes in staging
means, but I did look at it in my repo. Perhaps a link can be added to the issue template?