Skip to content

feat: Add GitHub Enterprise App installation repository management APIs - #3831

Merged
gmlewis merged 7 commits into
google:masterfrom
nithish-95:issues-3829-p2
Dec 2, 2025
Merged

feat: Add GitHub Enterprise App installation repository management APIs#3831
gmlewis merged 7 commits into
google:masterfrom
nithish-95:issues-3829-p2

Conversation

@nithish-95

Copy link
Copy Markdown
Contributor

Description

Implements enterprise GitHub App installation repository management endpoints for issue #3829.

This PR adds the following endpoints:

  • GET /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories
  • PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories
  • PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add
  • PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/remove

Changes

  • Added ListRepositoriesForOrgInstallation method to list repositories accessible to an app installation
  • Added ToggleInstallationRepositories method to update repository selection
  • Added AddRepositoriesToInstallation method to add repositories to an installation
  • Added RemoveRepositoriesFromInstallation method to remove repositories from an installation
  • Added corresponding test cases for all methods
  • Added necessary request/response types

Testing

  • All unit tests pass
  • Linter checks pass
  • Follows existing code patterns in the repository

cc @gmlewis@Not-Dhananjay-Mishra

Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
@codecov

codecovBot commented Nov 16, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.44%. Comparing base (3afe183) to head (5d9326e).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #3831 +/- ##
==========================================
+ Coverage 92.42% 92.44% +0.02% 
==========================================
Files 197 198 +1 Lines 14167 14210 +43 ==========================================
+ Hits 13094 13137 +43 
Misses 884 884 Partials 189 189 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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, @nithish-95!
One minor tweak, please, then we should be ready to merge after a second LGTM+Approval from any other contributor to this repo.

cc: @stevehipwell - @alexandear - @zyfy29

Comment threadgithub/enterprise_apps.go Outdated
@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label Nov 17, 2025
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
@stevehipwell

Copy link
Copy Markdown
Contributor

How does this PR relate to #3830? They look related to me, which means we likely want to align the implementation including naming. Would it not make sense to wait for #3830 to be merged before reviewing this PR?

@nithish-95

Copy link
Copy Markdown
ContributorAuthor

I've rebased on master to use the existing AccessibleRepository struct and removed the duplicate definition from this PR.
I also updated ListRepositoriesForOrgInstallation to return []*AccessibleRepository and fixed ToggleInstallationRepositories to return *Installation as requested.

I feel sorry for the way I use Git!!

cc: @gmlewis - @Not-Dhananjay-Mishra - @stevehipwell

@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, @nithish-95!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra

Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated

@stevehipwellstevehipwell left a comment

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.

I've added some naming suggestions, happy to discuss the "why".

Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
@nithish-95

nithish-95 commented Nov 25, 2025

Copy link
Copy Markdown
ContributorAuthor

I've added some naming suggestions, happy to discuss the "why".

@stevehipwell Yes, Please!

@stevehipwell

Copy link
Copy Markdown
Contributor

@nithish-95 which parts of the review did you want to discuss?

@nithish-95

nithish-95 commented Nov 27, 2025

Copy link
Copy Markdown
ContributorAuthor

@nithish-95 which parts of the review did you want to discuss?

Hi @stevehipwell , I'm making those updates.

Could you briefly explain the rationale behind these specific suggestions? I'm particularly interested in the conventions for type scoping for Enterprise prefixing and the preferred usage of AppInstallation vs Installation.

- Use EnterpriseService instead of creating new service
- Add EnterpriseInstallationRepositoriesOptions for consistency
- Update method signatures to Required
…tead of ListRepositories and adjust its test expectations.
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
Comment threadgithub/enterprise_apps.go Outdated
@stevehipwell

Copy link
Copy Markdown
Contributor

@nithish-95 check out the naming sections in the Google Go Style guide and the Google Go Style Decisions.

@Not-Dhananjay-MishraNot-Dhananjay-Mishra left a comment

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 @nithish-95
LGTM.

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @Not-Dhananjay-Mishra!

@stevehipwell - have your comments been addressed to your satisfaction and is this ready to merge?

@stevehipwellstevehipwell left a comment

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.

LGTM

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Dec 2, 2025
@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @stevehipwell!
Merging.

@gmlewis
gmlewis merged commit b480d82 into google:masterDec 2, 2025
7 checks passed
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

@nithish-95@stevehipwell@gmlewis@Not-Dhananjay-Mishra