Skip to content
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

Add ability to rename a repository: Rename-GitHubRepository #145

Merged
merged 6 commits into from
Mar 2, 2020
Merged

Add ability to rename a repository: Rename-GitHubRepository #145

merged 6 commits into from
Mar 2, 2020

Conversation

mtboren
Copy link
Contributor

@mtboren mtboren commented Jan 30, 2020

This adds the ability to easily rename a repository via the new cmdlet Rename-GitHubRepository, as suggested in #144 . As we see in the diff, this PR:

  • adds the new cmdlet
  • adds a test for the new cmdlet
  • adds a new contributor

Running PSScriptAnalyzer returns no issue (returned $null), and Pester test results were unchanged except that there is now one new, successful test.

How does this look as an add to this great module?

@msftclas
Copy link

msftclas commented Jan 30, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Thanks so much for doing this, Matt! Very much appreciated!
Thanks as well for writing the extra test and verifying ScriptAnalyzer results as well.

I have some minor feedback for you here to address, but overall, this looks solid. Looking forward to adding it!

GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Show resolved Hide resolved
GitHubRepositories.ps1 Show resolved Hide resolved
mtboren and others added 2 commits January 31, 2020 17:03
Updating the examples to be more uniform (and admittedly slightly less fun)

Co-Authored-By: Howard Wolosky <HowardWolosky@users.noreply.github.com>
…is ParamSet, fix style on curly brace, update call to JSON conversion
@mtboren
Copy link
Contributor Author

mtboren commented Feb 4, 2020

Howdy, @HowardWolosky -- wanted to be sure that you saw that these are now ready for your re-review. Let me know if anything else catches your eye.

Summary: I made the edits you pointed out, added another test, re-ran PSScriptAnalyzer, and re-ran the Pester tests. Yay.

Cheers

@HowardWolosky
Copy link
Member

Thanks for the update. Will review this PR later this week.

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Matt!
Great update. Thanks so much for handling all of that prior feedback. Just a few tiny further update requests, and then we'll be ready to merge to master.

GitHubRepositories.ps1 Show resolved Hide resolved
GitHubRepositories.ps1 Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
@mtboren
Copy link
Contributor Author

mtboren commented Feb 10, 2020

Yes, I'll get those in this week (back now from vacation)

@mtboren
Copy link
Contributor Author

mtboren commented Feb 13, 2020

Howdy again, @HowardWolosky -- These are now ready for your re-re-review. Let me know what else, if anything.

Summary: I made the edits you pointed out, re-ran PSScriptAnalyzer, and re-ran the Pester tests. Yay.

Cheers

@mtboren
Copy link
Contributor Author

mtboren commented Feb 26, 2020

Hello, @HowardWolosky

Just bumping this to make sure it's still somewhere on the radar. Looking forward to when there is time to return to this PR

Cheers

@HowardWolosky
Copy link
Member

Apologies for that. Missed your previous bump. Will get to the review this week. Thanks for your patience.

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Really minor remaining feedback. Otherwise, this is good to go.
I'm considering adding that confirmation logic to Remove-GitHubRepository as well, but it would be a breaking change for users, so I likely wouldn't do it until after we've hit 1.0 (a major version change).

GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
Tests/GitHubRepositories.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubRepositories.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubRepositories.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubRepositories.tests.ps1 Outdated Show resolved Hide resolved
Remove extraneous blank lines, rename vars to remove Hungarian notation

Co-Authored-By: Howard Wolosky <HowardWolosky@users.noreply.github.com>
@mtboren
Copy link
Contributor Author

mtboren commented Feb 29, 2020

Greets, @HowardWolosky -

Feedback incorporated/committed. Let me know if there are any other tidbits. Else, looking forward to the merge!

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

🎆 This is great. Thanks so much for the contribution, Matt! 🎆
Looking forward to seeing more contributions from you in the future.
I'll get an update with this change published today or tomorrow.
:shipit:

@HowardWolosky HowardWolosky merged commit 536b762 into microsoft:master Mar 2, 2020
@HowardWolosky
Copy link
Member

This has now been published. :shipit:

@mtboren
Copy link
Contributor Author

mtboren commented Mar 2, 2020

Woo-hoo! Thanks much for working through getting this added. I'm glad to have been able to help a bit.

And, hurray for the quick publish, too! See you 'round the community..

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.

None yet

3 participants