Skip to content

GitHubRepositories: Add New-GitHubRepositoryFromTemplate Function - #221

Merged
Howard Wolosky (HowardWolosky) merged 18 commits into
microsoft:masterfrom
X-Guardian:New-GitHubRepositoryFromTemplate
Jun 26, 2020
Merged

GitHubRepositories: Add New-GitHubRepositoryFromTemplate Function#221
Howard Wolosky (HowardWolosky) merged 18 commits into
microsoft:masterfrom
X-Guardian:New-GitHubRepositoryFromTemplate

Conversation

@X-Guardian

@X-GuardianSimon Heather (X-Guardian) commented Jun 4, 2020

Copy link
Copy Markdown
Contributor

Description

This PR adds a new function New-GitHubRepositoryFromTemplate to the GitHubRepositories module. This function creates a new GitHub repository from a specified template repository.

Parameters
NameTypeDescription
RepositoryNameMandatory StringName of the repository to be created.
TemplateOwnerNameMandatory StringOwner of the template repository.
TemplateRepositoryNameMandatory StringName of the template repository.
OwnerNameStringOwner of the repository to be created. If not specified, the DefaultOwnerName configuration property value will be used.
DescriptionStringA short description of the repository.
PrivateSwitchBy default, this repository will be created Public. Specify this to create a private repository.
AccessTokenStringIf provided, this will be used as the AccessToken for authentication with the REST Api. Otherwise, will attempt to use the configured value or will run unauthenticated.
NoStatusSwitchIf this switch is specified, long-running commands will run on the main thread with no commandline status update. When not specified, those commands run in the background, enabling the command prompt to provide status information. If not supplied here, the DefaultNoStatus configuration property value will be used.

Issues Fixed

References

Create a repository using a template

Checklist

  • You actually ran the code that you just wrote, especially if you did just "one last quick change".
  • Comment-based help added/updated, including examples.
  • Static analysis
    is reporting back clean.
  • New/changed code adheres to our coding guidelines.
  • Changes to the manifest file follow the manifest guidance.
  • Unit tests were added/updated and are all passing. See testing guidelines.
  • Relevant usage examples have been added/updated in USAGE.md.
  • If desired, ensure your name is added to our Contributors list

@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

Can we trigger the CI for this PR?

@HowardWolosky

Copy link
Copy Markdown
Contributor

I'll kick off a CI build a little bit later today after the current one ends.

@HowardWolosky

Copy link
Copy Markdown
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

Can we trigger the CI for this PR?

@HowardWolosky

Copy link
Copy Markdown
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

A weird CI failure on MacOS for the static code analysis step:

2|$results=Invoke-ScriptAnalyzer-Path ./ –Recurse
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Collection was modified; enumeration operation may not execute.

May be related to this: Collection was modified; enumeration operation may not execute when running Invoke-ScriptAnalyzer

can you run the CI again and see if it is consistent?

@HowardWolosky

Copy link
Copy Markdown
Contributor

Yeah, I've been seeing that pop up from time to time. I've just been waiting out a fix for that bug.

@HowardWolosky

Copy link
Copy Markdown
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@X-Guardian
Simon Heather (X-Guardian) marked this pull request as ready for review June 12, 2020 22:53
@bergmeister

Christoph Bergmeister (bergmeister) commented Jun 13, 2020

Copy link
Copy Markdown
Contributor

Howard Wolosky (@HowardWolosky) Can you please open a new issue in the PSScriptAnalyzer repo with steps to repro: https://github.com/PowerShell/PowerShell/issues/new?assignees=&labels=Issue-Question&template=Bug_Report.md&title=My+bug+report
The referenced bug PowerShell/PSScriptAnalyzer#1516 that you mention is specific to the new 1.19.0 release and only when using the PSUseCorrectCasing rule that is not run by default by Invoke-ScriptAnalyzer. Therefore your issue is likely a completely different one despite the error message being the same. I cloned this branch locally and cannot reproduce unfortunately :-(
I opened PR #236 to help you log and report the error the next time.

Howard Wolosky (HowardWolosky) pushed a commit that referenced this pull request Jun 15, 2020
…rove ScriptAnalyzer installation to not skip publisher and check and not allow clobber (#236)
#221 reported that this repo causes PSScriptAnalyzer to sporadically throw an error.
In order to report the full error needed for analysis, report the full stack trace. In order to make this easier, I've updated `Invoke-ScriptAnalyzer` to run within a `try/catch using -ErrorAction Stop`, and report any error found with `$_.Exception.StackTrace` before re-throwing the error.
@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

Can we trigger the CI for this PR?

@HowardWoloskyHoward Wolosky (HowardWolosky) added api-repositories Work to complete the API's defined here: https://developer.github.com/v3/repos/ enhancement An issue or pull request introducing new functionality to the project. api completeness This is basic API functionality that hasn't been implemented yet. and removed enhancement An issue or pull request introducing new functionality to the project. labels Jun 18, 2020
@HowardWolosky

Copy link
Copy Markdown
Contributor

Can we trigger the CI for this PR?

I can once the conflicts stemming from #242 have been resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this functionality!

Some feedback is included for you to review.

Comment threadGitHubRepositories.ps1
Comment threadGitHubRepositories.ps1 Outdated
Comment threadGitHubRepositories.ps1
Comment threadGitHubRepositories.ps1
Comment threadGitHubRepositories.ps1
Comment threadTests/GitHubRepositories.tests.ps1 Outdated
Comment threadTests/GitHubRepositories.tests.ps1 Outdated
Comment threadTests/GitHubRepositories.tests.ps1 Outdated
@HowardWoloskyHoward Wolosky (HowardWolosky) added the waiting for update Waiting for an update to the PR before the next review label Jun 20, 2020
@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

Can we trigger the CI for this PR?

@HowardWolosky

Copy link
Copy Markdown
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Super minor additional changes, and then we can get this merged. Thanks for the new feature!

Comment threadGitHubRepositories.ps1 Outdated
Comment threadGitHubRepositories.ps1 Outdated
Comment threadGitHubRepositories.ps1
Comment threadTests/GitHubRepositories.tests.ps1
Comment threadTests/GitHubRepositories.tests.ps1
Comment threadTests/GitHubRepositories.tests.ps1
Comment threadUSAGE.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just noticed one bug before I was about to run CI. I'll kick off CI and then merge this in once that gets fixed.

Comment threadGitHubRepositories.ps1 Outdated
Comment threadGitHubRepositories.ps1 Outdated
Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>
Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>
@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

Howard Wolosky (@HowardWolosky), ready for CI.

@HowardWolosky

Copy link
Copy Markdown
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@X-Guardian

Copy link
Copy Markdown
ContributorAuthor

Howard Wolosky (@HowardWolosky) , can we trigger the CI for this PR?

@HowardWolosky

Copy link
Copy Markdown
Contributor

Sure thing. Will kick it off once the current one completes, and then get this merged in.

@HowardWoloskyHoward Wolosky (HowardWolosky) removed the waiting for update Waiting for an update to the PR before the next review label Jun 26, 2020
@HowardWolosky

Copy link
Copy Markdown
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@HowardWolosky
Howard Wolosky (HowardWolosky) merged commit d96541e into microsoft:masterJun 26, 2020
@HowardWolosky

Copy link
Copy Markdown
Contributor

Simon Heather (@X-Guardian) - looks like this work doesn't fully support pipeline input, and the tests aren't sufficiently figuring that out either.

You're not capturing $RepositoryName = $elements.repositoryName at the top, which means that input from $Uri will never resolve correctly.

$elements=Resolve-RepositoryElements-BoundParameters $PSBoundParameters
$OwnerName=$elements.ownerName
$telemetryProperties=@{

  1. You're passing in -WhatIf which meant that you missed the bug in the method, since the failure was only seen as an exception thrown from the GitHub API itself when RepositoryName was missing in the uri fragment itself which won't occur due to the -WhatIf (the pipeline input needs to be a real separate test):

{ $templateRepo|New-GitHubRepositoryFromTemplate@newGitHubRepositoryFromTemplateParms-WhatIf } |
Should -Not-Throw

  1. Once you make those changes, I'm seeing some additional failures that start to occur.

Want to take a crack at a fix?

Howard Wolosky (HowardWolosky) added a commit to HowardWolosky/PowerShellForGitHub that referenced this pull request Jun 30, 2020
Added in microsoft#221, `New-GitHubRepositoryFromTemplate was not capturing
the passed-in value for the RepositoryName if provided via a Uri
parameter.
There was a pipeline test in place, however it missed this fact
because it was only testing the pipeline input scenario using `-WhatIf`,
and the error was only found when calling the actual REST API and
GitHub noticed that the `RepositoryName` was missing from the URI.
@HowardWolosky

Copy link
Copy Markdown
Contributor

I put out a fix with #259

Howard Wolosky (HowardWolosky) added a commit that referenced this pull request Jun 30, 2020
…#259)
Added in #221, `New-GitHubRepositoryFromTemplate was not capturing the passed-in value for the RepositoryName if provided via a Uri parameter.
There was a pipeline test in place, however it missed this fact because it was only testing the pipeline input scenario using `-WhatIf`, and the error was only found when calling the actual REST API and GitHub noticed that the `RepositoryName` was missing from the URI.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api completenessThis is basic API functionality that hasn't been implemented yet.api-repositoriesWork to complete the API's defined here: https://developer.github.com/v3/repos/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New Function Proposal: New-GitHubRepositoryFromTemplate

3 participants

@X-Guardian@HowardWolosky@bergmeister