Skip to content

Commit 17f6122

Browse files
Adding complete Pipeline support to the entire module (#242)
# Description Comprehensively adds pipeline support throughout the entire module This change required examining every method, and in some cases a few breaking changes (noted below) had to be introduced in order to make support work consistently end-to-end. UT's were added for every function, which resuled in a few bugs that were identified and fixed (also noted below). ## Breaking changes ### Unavoidable Breaks * `Get-GitHubRepositoryBranch`: `Name` parameter is now `BranchName`. > A `GitHub.Repository` object has a `name` property which is the name of the repo. Piping in the repo object to this method would confuse it, making it search for a branch with the name of the repo, as opposed to the desired effect of just listing out all branches in the repo. * `*-GitHub*Label`: `Name` parameter is now `Label`. > Similar to above. > `Name` was too generic and was causing unintended conflicts when passing in certain objects over the pipeline (like a `GitHub.Repository`) which also has a `name` property, making it impossible to pipe in a repository to list all labels (since it was trying to query for a label in that repository named the same as the repo. * `*-GitHubLabel`: `Milestone` parameter is now `MilestoneNumber`. > A `GitHub.Issue` contains a `milestone` object property, and PowerShell was complaining that there was no way to convert that to an `[int64]` when an Issue was passed in via the pipeline. The only way to avoid this was to use `MilestoneNumber` which is the name of the property we add to `GitHub.Milestone` objects, and it's what we alias to in all other methods that interact with milestones. * `*-GitHubIssue`: `Milestone` parameter is now `MilestoneNumber`. > A `GitHub.Issue` contains a `milestone` object property, and PowerShell was complaining that there was no way to convert that to an `[int64]` when an Issue was passed in via the pipeline. The only way to avoid this was to use `MilestoneNumber` which is the name of the property we add to `GitHub.Milestone` objects, and it's what we alias to in all other methods that interact with milestones. * `Get-GitHubLicense`: `Name` parameter is now `Key`. > The `key` is what you can query against, but a License object has _both_ a `name` and a `key` property, which caused piped object queries to fail with the existing parameter name. * `Get-GitHubCodeOfConduct`: `Name` parameter is now `Key`. > Similar to above. > The `key` is what you can query against, but a Code of Conduct object has _both_ a `name` and a `key` property, which caused piped object queries to fail with the existing parameter name. * `New-ProjectCard`: Signature has changed. Instead of specifying `ContentId` and `ContentType`, you now just directly specify either `IssueId` or `PullRequestId`. > Pipeline input would not have worked without this change. Additionally, this simply provides a cleaner interface for users in general, requiring one less input. * `Get-GitHubUserContextualInformation`: Signature has changed. Instead of specifying `SubjectId` and `SubjectType`, you now just directly specify either `OrganizationId`, `RepositoryId`, `IssueId` or `PullRequestId`. > Similar to above. > Pipeline input would not have worked without this change. Additionally, this simply provides a cleaner interface for users in general, requiring one less input. ### Breaks With Workarounds * A number of changes to the Comments functions * `GitHubComments.ps1` was renamed to `GitHubIssueComments.ps1` given that there are 5 different types of comments, these functions focus on `Issue` comments, and there doesn't currently appear to be a path forward for creating a single unified API set for all comment types. * All of these methods were renamed from `*-GitHubComment` to `*-GitHubIssueComment`, however they were also aliased back to their previous names (for now). You should really migrate to these new names however. * The main parameter was renamed from `CommentId` to `Comment`, however it was aliased back to `CommentId`. * `*-GitHubProject`: `ArchivedState` parameter is now `State` (but aliased back to `ArchivedState` to help with migration). * `*-GitHubProject`: `Name` parameter is now `ColumnName` (but aliased back to `Name`) * `*-GitHubProjectColumn`: `Name` parameter is now `ColumnName` (but aliased back to `Name`) * `Move-GitHubProjectCard`: `ColumnId` parameter is now `Column` (but aliased back to `ColumnId` to support pipeline input). * `Get-GitHubRelease`: `ReleaseId` parameter is now `Release` (but aliased back to `ReleaseId` to support pipeline input). * `Rename-GitHubRepository` has been effectively deprectaed. It was built on top of the same endpoint that `Set-GitHubRepository` uses, but was only modifying a single property (the name). For now, the function remains, but it now just internally calls into `Set-GitHubRepository` to perform the same action. * `Set-GitHubRepositoryTopic`: `Name` parameter is now `Topic` (but aliased back to `Name`). * `Get-GitHubUser`: `User` parameter is now `UserName` (but aliased back to `User` and `Name`). * `Get-GitHubUserContextualInformation`: `User` parameter is now `UserName` (but aliased back to `User` and `Name`). ## Bug Fixes: * Events had been using a typo'd version of the `sailor-v` AcceptHeader. This was fixed as part of the migration to using the AcceptHeader constants. * `Lock-GitHubIssue` was not correctly providing the `lock_reason` value to the API, so that metadata was getting lost. This was caught by new UT's and then fixed. * `Update-GitHubLabel` was modified to accept colors both with and without a leading # sign, just like `New-GitHubLabel` already supported. ## New Features: * `Get-GitHubGitIgnore` has a new `RawContent` switch which allows you to directly get back the content of the actual .gitignore file. * `Set-GitHubRepository` has been updated to allow users to change the name (rename) the repository at the same time as making any of their other changes. If a new name has been specified, users will be required to confirm the change, or to specify `-Confirm:$false` and/or `-Force` to avoid the confirmation. * `Get-GitHubRepositoryCollaborator` has gained a new parameter `Affiliation` allowing you to filter which collaborators you're interested in querying for. ## Minor updates: * Fixed the casing of the "microsoft" OwnerName in any examples * Fixed the casing of `GitHub` in the `Assignee` method names (was `Github` intead of `GitHub` * Added new configuration property (`DisablePipelineSupport`) to assist with pipeline development * All `AcceptHeader` usage has migrated to using script-wide constants * `Split-GitHubUri` can now return both components in a single function call if no switch is specified. * Added `Join-GitHubUri` which can reverse what `Split-GitHubUri` does, based on the configured `ApiHostName`. * Adds type information (via `.OUTPUTS` and `[OutputType()]` for every function. * Documentation updates reflecting the new pipeline support. ## Test changes: * `Set-StrictMode -Version 1.0` has been specified for all tests to catch access to undeclared variables (which should help catch common typo errors) * The workarounds for PSScriptAnalyzer's false complaint for rule `PSUseDeclaredVarsMoreThanAssignments` have been removed, and all test files now suppress that rule. * A test file now exists for every module file, and some tests were moved around to the appropriate file (away from GitHubAnalytics.tests.ps1 which had previously been a dumping ground for tests). * A _substantial_ number of new tests have been added. Every function should now be tested with limited exceptions (like for Organizations and Teams where we don't yet have the support in the module to get the account in the appropriate state to be able to query with those existing functions). > Note: These tests fully pass with Pester 4.10.1. Some additional work may be done prior to merging to get them passing with Pester 5.0, but the priority is to get this merged into master soon in order to unblock the pipeline of existing pull requests that have been waiting on this change. #### Issues Fixed - Fixes #63 #### References [The whole API set](https://developer.github.com/v3/)
1 parent 1f63817 commit 17f6122

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+9752
-2112
lines changed

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@
3232
-->
3333
- [ ] You actually ran the code that you just wrote, especially if you did just "one last quick change".
3434
- [ ] Comment-based help added/updated, including examples.
35-
- [ ] [Static analysis](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#static-analysis)
36-
is reporting back clean.
35+
- [ ] [Static analysis](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#static-analysis) is reporting back clean.
3736
- [ ] New/changed code adheres to our [coding guidelines](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#coding-guidelines).
37+
- [ ] New/changed code continues to [support the pipeline](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#pipeline-support).
3838
- [ ] Changes to the manifest file follow the [manifest guidance](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#module-manifest).
39-
- [ ] Unit tests were added/updated and are all passing. See [testing guidelines](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#testing).
39+
- [ ] Unit tests were added/updated and are all passing. See [testing guidelines](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#testing). This includes making sure that all pipeline input variations have been covered.
4040
- [ ] Relevant usage examples have been added/updated in [USAGE.md](https://github.com/microsoft/PowerShellForGitHub/blob/master/USAGE.md).
4141
- [ ] If desired, ensure your name is added to our [Contributors list](https://github.com/microsoft/PowerShellForGitHub/blob/master/CONTRIBUTING.md#contributors)

CONTRIBUTING.md

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Looking for information on how to use this module? Head on over to [README.md](
2323
* [Adding New Configuration Properties](#adding-new-configuration-properties)
2424
* [Code Comments](#code-comments)
2525
* [Debugging Tips](#debugging-tips)
26+
* [Pipeline Support](#pipeline-support)
2627
* [Testing](#testing)
2728
* [Installing Pester](#installing-pester)
2829
* [Configuring Your Environment](#configuring-your-environment)
@@ -286,8 +287,71 @@ Set-GitHubConfiguration -LogRequestBody
286287

287288
----------
288289

290+
### Pipeline Support
291+
292+
This module has comprehensive support for the PowerShell pipeline. It is imperative that all
293+
new functionality added to the module embraces this design.
294+
295+
* Most functions are declared as a `filter`. This is the equivalent of a `function` where the
296+
body of the function is the `process` block, and the `begin/end` blocks are empty.
297+
298+
* In limited cases where one of the inputs is an array of something, and you specifically want that
299+
to be processed as a single command (like adding a bunch of labels to a single issue at once),
300+
you can implement it as a `function` where you use `begin/process` to gather all of the values
301+
into a single internal array, and then do the actual command execution in the `end` block. A
302+
good example of that which you can follow can be seen with `Add-GitHubIssueLabel`.
303+
304+
* Any function that requires the repo's `Uri` to be provided should be additionally aliased with
305+
`[Alias('RepositoryUrl')]` and its `[Parameter()]` definition should include `ValueFromPipelineByPropertyName`.
306+
307+
* Do not use any generic term like `Name` in your parameters. That will end up causing unintended
308+
pipeline issues down the line. For instance, if it's a label, call it `Label`, even though `Name`
309+
would make sense, other objects in the pipeline (like a `GitHub.Respository` object) also have
310+
a `name` property that would conflict.
311+
312+
* You should plan on adding additional properties to all objects being returned from an API call.
313+
Any object that is specific to a repository should have a `RepositoryUrl` `NoteProperty` added
314+
to it, enabling it to be piped-in to any other command that requires knowing which repository
315+
you're talking about. Additionally, any other property that might be necessary to uniquely
316+
identify that object in a different command should get added properties. For example, with Issues,
317+
we add both an `IssueNumber` property and an `IssueId` property to it, as the Issue commands
318+
need to interact with the `IssueNumber` while the Event commands interact with the `IssueId`.
319+
We prefer to _only_ add additional properties that are believed to be needed as input to other
320+
commands (as opposed to creating alias properties for all of the object's properties).
321+
322+
* For every major file, you will find an `Add-GitHub*AdditionalProperties` filter method at the end.
323+
If you're writing a new file, you'll need to create this yourself (and model it after an existing
324+
one). The goal of this is that you can simply pipe the output of your `Invoke-GHRestMethod`
325+
directly into this method to update the result with the additional properties, and then return
326+
that modified version to the user. The benefit of this approach is that you can then apply that
327+
filter on child objects within the primary object. For instance, a `GitHub.Issue` has multiple
328+
`GitHub.User` objects, `GitHub.Label` objects, a `GitHub.Milestone` object and more. Within
329+
`Add-GitHubIssueAdditionalProperties`, it just needs to know to call the appropriate
330+
`Add-GitHub*AdditionalProperties` method on the qualifying child properties, without needing to
331+
know anything more about them.
332+
333+
* That method will also "type" information to each object. This is forward-looking work to ease
334+
support for providing formatting of various object types in the future. That type should be
335+
defined at the top of the current file at the script level (see other files for an example),
336+
and you should be sure to both specify it in the `.OUTPUTS` section of the Comment Based Help (CBH)
337+
for the command, as well as with `[OutputType({$script:GitHubUserTypeName})]` (for example).
338+
339+
* Going along with the `.OUTPUTS` is the `.INPUTS` section. Please maintain this section as well.
340+
If you add any new type that will gain a `RepositoryUrl` property, then you'll need to update
341+
virtually _all_ of the `.INPUTS` entries across all of the files where the function has a `Uri`
342+
parameter. Please keep these type names alphabetical.
343+
344+
* To enable debugging issues involving pipeline support, there is an additional configuration
345+
property that you might use: `Set-GitHubConfiguration -DisablePipelineSupport`. That will
346+
prevent the module from adding _any_ additional properties to the objects.
347+
348+
----------
349+
289350
### Testing
290351
[![Build status](https://dev.azure.com/ms/PowerShellForGitHub/_apis/build/status/PowerShellForGitHub-CI?branchName=master)](https://dev.azure.com/ms/PowerShellForGitHub/_build/latest?definitionId=109&branchName=master)
352+
[![Azure DevOps tests](https://img.shields.io/azure-devops/tests/ms/PowerShellForGitHub/109/master)](https://dev.azure.com/ms/PowerShellForGitHub/_build/latest?definitionId=109&branchName=master)
353+
[![Azure DevOps coverage](https://img.shields.io/azure-devops/coverage/ms/PowerShellForGitHub/109/master)](https://dev.azure.com/ms/PowerShellForGitHub/_build/latest?definitionId=109&branchName=master)
354+
291355

292356
#### Installing Pester
293357
This module supports testing using the [Pester UT framework](https://github.com/pester/Pester).
@@ -350,6 +414,8 @@ There are many more nuances to code-coverage, see
350414

351415
#### Automated Tests
352416
[![Build status](https://dev.azure.com/ms/PowerShellForGitHub/_apis/build/status/PowerShellForGitHub-CI?branchName=master)](https://dev.azure.com/ms/PowerShellForGitHub/_build/latest?definitionId=109&branchName=master)
417+
[![Azure DevOps tests](https://img.shields.io/azure-devops/tests/ms/PowerShellForGitHub/109/master)](https://dev.azure.com/ms/PowerShellForGitHub/_build/latest?definitionId=109&branchName=master)
418+
[![Azure DevOps coverage](https://img.shields.io/azure-devops/coverage/ms/PowerShellForGitHub/109/master)](https://dev.azure.com/ms/PowerShellForGitHub/_build/latest?definitionId=109&branchName=master)
353419

354420
These test are configured to automatically execute upon any update to the `master` branch
355421
of `microsoft/PowerShellForGitHub`.
@@ -362,9 +428,9 @@ as well...it is stored, encrypted, within Azure DevOps. It is not accessible fo
362428
the CI pipeline. To run the tests locally with your own account, see
363429
[configuring-your-environment](#configuring-your-environment).
364430

365-
> NOTE: We're currently encountering issues with the tests successfully running within the pipeline.
366-
> They do complete successfully locally, so please test your changes locally before submitting a
367-
> pull request.
431+
> Your change must successfully pass all tests before they will be merged. While we will run a CI
432+
> build on your behalf for any submitted pull request, it's to your benefit to verify your changes
433+
> locally first.
368434
369435
#### New Test Guidelines
370436
Your tests should have NO dependencies on an account being set up in a specific way. They should

GitHubAnalytics.ps1

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,12 @@ function Group-GitHubIssue
2424
The date property that should be inspected when determining which week grouping the issue
2525
if part of.
2626
27+
.INPUTS
28+
GitHub.Issue
29+
2730
.OUTPUTS
28-
[PSCustomObject[]] Collection of issues and counts, by week, along with the total count of issues.
31+
[PSCustomObject[]]
32+
Collection of issues and counts, by week, along with the total count of issues.
2933
3034
.EXAMPLE
3135
$issues = @()
@@ -90,8 +94,12 @@ function Group-GitHubIssue
9094
foreach ($week in $weekDates)
9195
{
9296
$filteredIssues = @($Issue | Where-Object {
93-
(($DateType -eq 'Created') -and ($_.created_at -ge $week) -and ($_.created_at -le $endOfWeek)) -or
94-
(($DateType -eq 'Closed') -and ($_.closed_at -ge $week) -and ($_.closed_at -le $endOfWeek))
97+
(($DateType -eq 'Created') -and
98+
($_.created_at -ge $week) -and
99+
($_.created_at -le $endOfWeek)) -or
100+
(($DateType -eq 'Closed') -and
101+
($_.closed_at -ge $week) -and
102+
($_.closed_at -le $endOfWeek))
95103
})
96104

97105
$endOfWeek = $week
@@ -144,6 +152,9 @@ function Group-GitHubPullRequest
144152
The date property that should be inspected when determining which week grouping the
145153
pull request if part of.
146154
155+
.INPUTS
156+
GitHub.PullRequest
157+
147158
.OUTPUTS
148159
[PSCustomObject[]] Collection of pull requests and counts, by week, along with the
149160
total count of pull requests.
@@ -211,8 +222,12 @@ function Group-GitHubPullRequest
211222
foreach ($week in $weekDates)
212223
{
213224
$filteredPullRequests = @($PullRequest | Where-Object {
214-
(($DateType -eq 'Created') -and ($_.created_at -ge $week) -and ($_.created_at -le $endOfWeek)) -or
215-
(($DateType -eq 'Merged') -and ($_.merged_at -ge $week) -and ($_.merged_at -le $endOfWeek))
225+
(($DateType -eq 'Created') -and
226+
($_.created_at -ge $week) -and
227+
($_.created_at -le $endOfWeek)) -or
228+
(($DateType -eq 'Merged') -and
229+
($_.merged_at -ge $week) -and
230+
($_.merged_at -le $endOfWeek))
216231
})
217232

218233
$endOfWeek = $week
@@ -265,6 +280,7 @@ function Get-WeekDate
265280
Get-WeekDate -Weeks 10
266281
#>
267282
[CmdletBinding()]
283+
[OutputType([DateTime[]])]
268284
param(
269285
[ValidateRange(0, 10000)]
270286
[int] $Weeks = 12

0 commit comments

Comments
 (0)