Skip to content

Conversation

@EyalDelarea
Copy link
Contributor

@EyalDelarea EyalDelarea commented May 25, 2025

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • I used go fmt ./... for formatting the code before submitting the pull request.
  • This feature is included on all supported VCS providers - GitHub, Bitbucket cloud, Bitbucket server, GitLab and Azure Repos.
  • I added the relevant documentation for the new feature.

✨ Add ListAppRepositories Function to VCS Client Interface

This PR introduces a new function ListAppRepositories to the VcsClient interface, currently implemented only for GitHub.

✅ What It Does
• Adds support for fetching GitHub App-accessible repositories using the installation/repositories endpoint.

⚙️ Why Only GitHub?

Currently, GitHub is the only provider where:
• App-based repo access is a common pattern (via installations).
• There’s a clear, supported API endpoint for listing them.

The interface was designed to support future implementations for GitLab, Bitbucket, etc., once similar functionality is available or required.

📘 Usage Example

ctx := context.Background()
appRepositories, err := client.ListAppRepositories(ctx)
for _, repo := range appRepositories {
    fmt.Println(repo.FullName, repo.CloneURL)
}

@EyalDelarea EyalDelarea added the new feature Automatically generated release notes label May 27, 2025
@EyalDelarea EyalDelarea changed the title Add app repos list Add ListAppRepositories method to VCS May 27, 2025
@EyalDelarea EyalDelarea marked this pull request as ready for review May 27, 2025 09:33
@EyalDelarea EyalDelarea requested review from eyalk007 and orto17 May 27, 2025 09:33
@eyalk007 eyalk007 requested review from attiasas and eranturgeman May 27, 2025 15:51
ListRepositories(ctx context.Context) (map[string][]string, error)

// ListAppRepositories ListRepositories Returns a map between all accessible App to their list of repositories
ListAppRepositories(ctx context.Context) ([]AppRepositoryInfo, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

What identifies a repository as an "App repository"? what is passed in the context that prevents getting all existing repositories?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically it has more fields and context then our basic repository object.
this is again to not interfere with the current objects and logic and trying to separate them as much as possible for now.

I agree that they could be merged, but as this is changing by the demands, i wanted to keep them separated for now at least.

SSHURL: repo.GetSSHURL(),
DefaultBranch: repo.GetDefaultBranch(),
}
results = append(results, repoInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

please ensure we cannot get nil pointer dereference (since you declare the results but doesn't init is using 'make')

Copy link
Contributor

@eranturgeman eranturgeman left a comment

Choose a reason for hiding this comment

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

Looks good! please see my notes

@EyalDelarea EyalDelarea added improvement Automatically generated release notes and removed new feature Automatically generated release notes labels May 28, 2025
@EyalDelarea
Copy link
Contributor Author

@eranturgeman if you can merge this once you are ready.

@eranturgeman eranturgeman self-requested a review June 17, 2025 07:32
@github-actions
Copy link
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@eranturgeman eranturgeman merged commit 82b7206 into jfrog:master Jun 17, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Automatically generated release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants