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.

Sign up for GitHub

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

Jump to bottom

GitHubRepositories: Add Get/Set GitHub Repository Actions Permissions #301

Merged
merged 5 commits into from
Dec 6, 2020
Merged

GitHubRepositories: Add Get/Set GitHub Repository Actions Permissions #301

merged 5 commits into from
Dec 6, 2020

Conversation

X-Guardian
Copy link
Contributor

@X-Guardian X-Guardian commented Nov 7, 2020 •
edited by HowardWolosky
Loading

Description

This PR adds the following functions to the GitHubRepositories module:

  • Get-GitHubRepositoryActionsPermission
  • Set-GitHubRepositoryActionsPermission

Issues Fixed

None

References

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.
  • Formatters were created for any new types being added.
  • New/changed code continues to support the pipeline.
  • Changes to the manifest file follow the manifest guidance.
  • Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure that all pipeline input variations have been covered.
  • Relevant usage examples have been added/updated in USAGE.md.
  • If desired, ensure your name is added to our Contributors list

@X-Guardian X-Guardian marked this pull request as ready for review November 7, 2020 17:43
@HowardWolosky HowardWolosky added api completeness This is basic API functionality that hasn't been implemented yet. api-repositories Work to complete the API's defined here: https://developer.github.com/v3/repos/ labels Nov 8, 2020
HowardWolosky
HowardWolosky requested changes Dec 2, 2020
View reviewed changes
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 again for this submission, and apologies for the delayed review.

Some minor feedback to address and then we should be good to merge in.

GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated
[string] $Uri,

[Parameter(Mandatory)]
[ValidateSet('All', 'Local_Only', 'Selected', 'Disabled')]
Copy link
Member

Choose a reason for hiding this comment

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

While the API uses local_only, the whole underscore thing isn't very PowerShell-y. I'd prefer that to be LocalOnly with a conversion to local_only internally. In the past, we've used hashtable "converters" to handle scenarios like this. Example:

PowerShellForGitHub/GitHubIssues.ps1

Lines 900 to 901 in 8a29c7f

[ValidateSet('OffTopic', 'TooHeated', 'Resolved', 'Spam')]
[string] $Reason,

PowerShellForGitHub/GitHubIssues.ps1

Lines 923 to 931 in 8a29c7f

$reasonConverter = @{
'OffTopic' = 'off-topic'
'TooHeated' = 'too heated'
'Resolved' = 'resolved'
'Spam' = 'spam'
}
$telemetryProperties['Reason'] = $Reason
$hashBody['lock_reason'] = $reasonConverter[$Reason]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Outdated Show resolved Hide resolved
GitHubRepositories.ps1 Show resolved Hide resolved
@ghost ghost added the needs-author-feedback label Dec 2, 2020
@HowardWolosky
Copy link
Member

/azp run PowerShellForGitHub-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost ghost removed the needs-author-feedback label Dec 5, 2020
X-Guardian
X-Guardian commented Dec 5, 2020
View reviewed changes
Copy link
Contributor Author

@X-Guardian X-Guardian left a comment

Choose a reason for hiding this comment

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

@HowardWolosky, ready for the next review.

HowardWolosky
HowardWolosky previously approved these changes Dec 6, 2020
View reviewed changes
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 for the quick update. Looks like there was only one typo, but I'll get that suggested update applied myself once this review is submitted. Then I'll kick off a pipeline run and then get it submitted.

Thanks for your efforts here!

GitHubRepositories.ps1 Outdated Show resolved Hide resolved
@HowardWolosky
Copy link
Member

/azp run PowerShellForGitHub-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@HowardWolosky HowardWolosky merged commit d499705 into microsoft:master Dec 6, 2020
@X-Guardian X-Guardian deleted the GitHubRepositories-Actions-Permissions branch December 27, 2020 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HowardWolosky HowardWolosky HowardWolosky left review comments

Assignees
No one assigned
Labels
api completeness This is basic API functionality that hasn't been implemented yet. api-repositories Work to complete the API's defined here: https://developer.github.com/v3/repos/
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@X-Guardian @HowardWolosky