Skip to content

Add enterprise runner group operations - #2891

Merged
gmlewis merged 8 commits into
google:masterfrom
gabriel-samfira:add-enterprise-runner-group
Sep 6, 2023
Merged

Add enterprise runner group operations#2891
gmlewis merged 8 commits into
google:masterfrom
gabriel-samfira:add-enterprise-runner-group

Conversation

@gabriel-samfira

Copy link
Copy Markdown
Contributor

This change adds runner group operations on enterprises. The operations are essentially a mirror of the org runner group code, adapted for enterprises.

This change adds runner group operations on enterprises.
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
@codecov

codecovBot commented Aug 21, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2891 (428e724) into master (b700431) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## master #2891 +/- ##
==========================================
+ Coverage 98.12% 98.14% +0.02% 
==========================================
Files 142 143 +1 Lines 12453 12593 +140 ==========================================
+ Hits 12219 12359 +140 
Misses 159 159 Partials 75 75 
Files ChangedCoverage Δ
github/enterprise_actions_runner_groups.go100.00% <100.00%> (ø)

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @gabriel-samfira for the very clean and well-written PR!
❤️
Just one tiny nit, please, then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

Comment threadgithub/enterprise_actions_runner_groups.go Outdated
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @gabriel-samfira !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label Aug 21, 2023
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
@gabriel-samfira

Copy link
Copy Markdown
ContributorAuthor

I had forgotten about the omitempty tag. Added it now.

@WillAbides

Copy link
Copy Markdown
Contributor

For consistency with ActionsService, should the "*RunnerGroup" methods be renamed to "*EnterpriseRunnerGroup"?

This PR also takes the opposite approach with the "*GroupRunners" methods. ActionsService doesn't include "Organization" in those method names, but EnterpriseService includes "Enterprise". I think the ActionsService names make more sense because they are acting on a "Runner", not an "OrganizationRunner" or "EnterpriseRunner".

@gmlewis

Copy link
Copy Markdown
Collaborator

For consistency with ActionsService, should the "*RunnerGroup" methods be renamed to "*EnterpriseRunnerGroup"?

This PR also takes the opposite approach with the "*GroupRunners" methods. ActionsService doesn't include "Organization" in those method names, but EnterpriseService includes "Enterprise". I think the ActionsService names make more sense because they are acting on a "Runner", not an "OrganizationRunner" or "EnterpriseRunner".

Excellent points, @WillAbides - I think you are right.
@gabriel-samfira - would you mind making these naming changes, please?

@gabriel-samfira

Copy link
Copy Markdown
ContributorAuthor

Ohh wow. For some reason I did not get a notification for your comments. I will make these changes ASAP. Thanks for the review!

@gabriel-samfira

Copy link
Copy Markdown
ContributorAuthor

Hopefully I got all the names right. Let me know if I missed (or misunderstood) anything.

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @WillAbides !
Merging.

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 6, 2023
@gmlewis
gmlewis merged commit 4030e93 into google:masterSep 6, 2023
@ajschmidt8

Copy link
Copy Markdown

Appreciate the work here! My team will benefit from these changes as well.

Thanks, everyone.

@gabriel-samfira
gabriel-samfira deleted the add-enterprise-runner-group branch September 7, 2023 19:21
gmlewis pushed a commit to gmlewis/go-github that referenced this pull request Sep 19, 2023
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto 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.

4 participants

@gabriel-samfira@WillAbides@gmlewis@ajschmidt8