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

Feature: use go-git #1709

Open
Tracked by #1683
azeemshaikh38 opened this issue Mar 4, 2022 · 9 comments
Open
Tracked by #1683

Feature: use go-git #1709

azeemshaikh38 opened this issue Mar 4, 2022 · 9 comments
Assignees
Labels
kind/enhancement New feature or request Stale
Projects

Comments

@azeemshaikh38
Copy link
Contributor

Proposing that we use go-git for powering RepoClient APIs that require git-like features. Some advantages I see of using this:

  • A common implementation for ListFiles, GetFileContent, ListCommits and Search that can be used across any Git-like system (GitHub, GitLab or even local git repos).
  • Enables us to add support for things like tags (e.g run scorecard on --commit=v4.1.0)
  • Might provide better performance since we won't be uncompressing a tarball and some added API token efficiency.

Considering to start work on this (not a top priority for me, will work on it on the side). Just wanted to run it by folks to make sure no one has concerns. @laurentsimon @naveensrinivasan @justaugustus

@azeemshaikh38 azeemshaikh38 added the kind/enhancement New feature or request label Mar 4, 2022
@azeemshaikh38 azeemshaikh38 self-assigned this Mar 4, 2022
@naveensrinivasan
Copy link
Member

👍

@justaugustus justaugustus added this to Backlog in Scorecard via automation Mar 4, 2022
@justaugustus
Copy link
Member

@azeemshaikh38 -- What would you say is the primary motivation for go-git vs go-github?

Is it:

A common implementation ... that can be used across any Git-like system (GitHub, GitLab or even local git repos).

Are you thinking something like this?

// git/clients.go
type Git interface{
	ListFiles()
	GetFileContent()
	ListCommits()
	Search()
}

// git/gitlab.go
type Gitlab struct { ... }

// git/local.go
type Local struct { ... }

// git/github.go
type Github struct {
	http.RoundTripper
}
func NewGithub(rt *http.RoundTripper) *Github {
	return (go-github).github.NewClient(&http.Client{Transport: rt})
}

func (gh *Github) ListFiles() error {
	// go-github implementation of ListFiles()
}

Suggested in #1491 (comment):

@naveensrinivasan -- my team in Kubernetes Release Engineering maintains a GitHub package, which is pretty well-tested and would allow you to offload this.

Could you look around and see if some of this fits your needs?

https://github.com/kubernetes-sigs/release-sdk/blob/main/github/github.go

@azeemshaikh38
Copy link
Contributor Author

What would you say is the primary motivation for go-git vs go-github?

Not really go-git vs. go-github, but rather using go-git vs. our custom logic for handling git data. Something along these lines:

// clients/git/git.go
type Git struct {
  CloneRemote(urlToRemote string)
  OpenFrom(pathToLocalDir string)
  ListFiles()
  GetFileContent()
  ...
}

// clients/githubrepo/client.go
func (gh *GitHubClient) ListFiles() {
  return Git.ListFiles()
}

// clients/gitlab/client.go
func (gl *GitLabClient) ListFiles() {
  return Git.ListFiles()
}

// similar for clients/localdir

The implementations can choose to use these if they prefer, but its not enforced at the code-level.

@laurentsimon
Copy link
Contributor

no concerns on my side, looks like a good idea, so long it's well maintained (both seem to be the case)

@justaugustus
Copy link
Member

The implementations can choose to use these if they prefer, but its not enforced at the code-level.

@azeemshaikh38 -- Okay, cool! We're saying the same thing, modulo impl details like file locations.

Organizationally, what you have is better i.e., package/impl/impl.go

The only thing I'd nit about keeping from is:

type Git interface{ ... }

Git should be an Interface for a few reasons:

  • we can mock it
  • we can ensure client impls satisfy the interface
  • it allows consumers to write their own concrete implementations of it

I'd call this part of the scorecard API work: #1683

@azeemshaikh38
Copy link
Contributor Author

  • it allows consumers to write their own concrete implementations of it

Could you expand on that? What's the usecase for making this an interface? Note that there is no caller which passes Git to a callee (the usual pattern for needing an interface).

Did you mean something like this?

type Git struct {} // implements clients.RepoClient interface

@justaugustus
Copy link
Member

@azeemshaikh38 -- Just as an example, consider functional options:

type Git interface{ ... }

type Option func(*Git)

type WithTransport(rt http.RoundTripper) Option {
	return func(g *Git) {
		g.rt = rt
	}
}

type Github struct{ ... }

func NewGithub(...Option) { ... }

func main() {
	rt := roundtripper.New()
	gh := NewGithub(
		WithTransport(rt),
	)

	// Do GitHub stuff
}

^^ in the above, any type that satisfies the Git interface can now also take advantage of WithTransport()

Concrete example:

@azeemshaikh38
Copy link
Contributor Author

I kind of see what you are driving at. Let me start coding this up, it'll probably help me untangle this.

Copy link

github-actions bot commented Nov 2, 2023

This issue is stale because it has been open for 60 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request Stale
Projects
Scorecard
Backlog
Status: No status
Development

No branches or pull requests

5 participants