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 methods to add and remove repos for actions on org level #1997

Merged
merged 3 commits into from
Jul 15, 2021

Conversation

FloThinksPi
Copy link
Contributor

This commit adds the abillity to add one or more repositories to the
list of enabled repositories for github actions on org level.
It also adds the abillity to remove them from this list.
With this change we can now enable and disable repos to use github actions on org level.

The used endpoints for this are:
https://docs.github.com/en/rest/reference/actions#set-selected-repositories-enabled-for-github-actions-in-an-organization
https://docs.github.com/en/rest/reference/actions#disable-a-selected-repository-for-github-actions-in-an-organization

fixes #1995

This commit adds the abillity to add one or more repositories to the
list of enabled repositories for github actions on org level.
It also adds the abillity to remove them from this list.
With this change we can now enable and disable repos to use github actions on org level.

The used endpoints for this are:
https://docs.github.com/en/rest/reference/actions#set-selected-repositories-enabled-for-github-actions-in-an-organization
https://docs.github.com/en/rest/reference/actions#disable-a-selected-repository-for-github-actions-in-an-organization
@google-cla
Copy link

google-cla bot commented Jul 14, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Jul 14, 2021
@codecov
Copy link

codecov bot commented Jul 14, 2021

Codecov Report

Merging #1997 (8351b36) into master (465df60) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1997   +/-   ##
=======================================
  Coverage   97.86%   97.86%           
=======================================
  Files         107      107           
  Lines        6871     6897   +26     
=======================================
+ Hits         6724     6750   +26     
  Misses         81       81           
  Partials       66       66           
Impacted Files Coverage Δ
github/actions_runners.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 465df60...8351b36. Read the comment docs.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thanks, @FloThinksPi , but this PR appears to have a few problems.
Did you try out these two new methods before submitting the PR?

I ask because one of the signatures doesn't match the official documentation and I'm not sure how it could have worked.

github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners_test.go Outdated Show resolved Hide resolved
github/actions_runners_test.go Outdated Show resolved Hide resolved
@FloThinksPi
Copy link
Contributor Author

Woh thanks @gmlewis for the super quick review.
I just wanted to display the diff and must have opened it already. Didnt run it yet at all, just the tests.
I will include your suggestions and try it out.

@google-cla
Copy link

google-cla bot commented Jul 14, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

…n org level

Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@google-cla
Copy link

google-cla bot commented Jul 14, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@FloThinksPi
Copy link
Contributor Author

@googlebot I signed it!

@google-cla
Copy link

google-cla bot commented Jul 14, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@gmlewis
Copy link
Collaborator

gmlewis commented Jul 14, 2021

Hi @FloThinksPi - it looks like the tests need updating:

=== RUN   TestActionsService_AddEnabledReposInOrg
    actions_runners_test.go:411: request Body is {"selected_repository_ids":[123,1234]}
        , want {"repository_id":[123,1234]}
--- FAIL: TestActionsService_AddEnabledReposInOrg (0.00s)

@gmlewis
Copy link
Collaborator

gmlewis commented Jul 14, 2021

@googlebot I consent.

@google-cla google-cla bot added cla: yes Indication that the PR author has signed a Google Contributor License Agreement. and removed cla: no labels Jul 14, 2021
@gmlewis gmlewis requested a review from wesleimp July 14, 2021 17:54
Switch from Add to Set in method name since the behaviour is to set the
list of enabled repos absolutely.

As a replacement added the real AddEnabledReposInOrg by implementing:
https://docs.github.com/en/rest/reference/actions#enable-a-selected-repository-for-github-actions-in-an-organization
which adds just a single entry to the list.
@FloThinksPi FloThinksPi requested a review from gmlewis July 14, 2021 19:24
@FloThinksPi
Copy link
Contributor Author

FloThinksPi commented Jul 14, 2021

@gmlewis Yea all done now 👍🏼 . I also renamed the AddEnableReposInOrg method to SetEnabledReposInOrg since the used endpoint sets the list absolute. I thus added the https://docs.github.com/en/rest/reference/actions#enable-a-selected-repository-for-github-actions-in-an-organization endpoint which is now AddEnableReposInOrg which appends to the list of enabled repos.

All tests return ok and i tested all three methods against a GitHubEnterprise 3.0 Instance.

Are you fine with the method names ?
Should i squash the commits ?

@gmlewis
Copy link
Collaborator

gmlewis commented Jul 14, 2021

Should i squash the commits ?

You don't need to squash, as we only "Squash and merge" in this repo anyway.

I'll check the names now.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @FloThinksPi !
LGTM.

Awaiting second LGTM before merging.

@gmlewis
Copy link
Collaborator

gmlewis commented Jul 15, 2021

Thank you, @wesleimp !
Merging.

@gmlewis gmlewis merged commit c478f27 into google:master Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indication that the PR author has signed a Google Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Github Actions SetEnabledRepos functionality is missing
3 participants