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

migrate the pullrequest-init code to go-scm to handle multiple git providers? #1066

Closed
jstrachan opened this issue Jul 11, 2019 · 7 comments · Fixed by #1521
Closed

migrate the pullrequest-init code to go-scm to handle multiple git providers? #1066

jstrachan opened this issue Jul 11, 2019 · 7 comments · Fixed by #1521
Labels
kind/question Issues or PRs that are questions around the project or a particular feature

Comments

@jstrachan
Copy link

jstrachan commented Jul 11, 2019

It would be great to migrate the pullrequest-init code to support any git provider (bitbucket, gitlab, gitea et al). e.g. for #1000

We've been using this fork of go-scm to handle multiple git providers via a single unified API: https://github.com/jenkins-x/go-scm and its been working great.

e.g. we've ported the main prow ChatOps commands into a new webhook handle using it: https://github.com/jenkins-x/lighthouse/ and its working great for many git providers.

Its a small simple library with modest dependencies so is a drop in replacement for the https://github.com/google/go-github library which would then solve the multi-git provider support issue.

The generated JSON is close to the current JSON (a few minor changes though) but at least uses the same JSON schema whatever git provider is used.

e.g. here's some examples

To create the scm.Client we just need 3 strings; the driver name, optional server URL and oauth token...

// for github
client, err := factory.NewClient("github", "", oauthToken)

// some gitlab server
client, err := factory.NewClient("gitlab", "https://foo.com/bar", oauthToken)

How do folks think about migrating to this go-scm library?

@vdemeester vdemeester added the kind/question Issues or PRs that are questions around the project or a particular feature label Jul 11, 2019
@dlorenc
Copy link
Contributor

dlorenc commented Jul 11, 2019

cc @wlynch @bobcatfish

@wlynch
Copy link
Member

wlynch commented Jul 11, 2019

Thanks for raising this!

I'm not against sharing the common type here, but we'll probably need to make some changes to support everything we want.

The biggest concern I see is the lack of a common status enum, since it means pipelines will need to know the implementation specific statuses. For example:

GitHub Status GitHub Check Run GitLab
success success success
failure failed failed
pending queued pending
error action_required
in_progess running
canceled canceled
neutral
timed_out

@jstrachan few questions:

  1. What's the feature diff between jenkins-x/go-scm and drone/go-scm? What was missing in the original library? Anything blocking upstreaming the changes?
  2. AFAICT, there doesn't appear to be support for PR status updates? Am I reading this right or am I not looking in the right place? 🙃

@dlorenc
Copy link
Contributor

dlorenc commented Jul 31, 2019

Looks like we also need support for labels in go-scm.

@kav
Copy link

kav commented Oct 7, 2019

Any update here? Happy to contribute where needed. Would really love to use Tekton to manage Bitbucket PRs. Is the next step adding BB PR labels to the jenkins-x go-scm fork?

@wlynch
Copy link
Member

wlynch commented Oct 10, 2019

@kav Yes! That would be very helpful. jenkins-x/go-scm#21

@wlynch
Copy link
Member

wlynch commented Oct 10, 2019

I'll be starting work to replatform the PullRequest resource on go-scm soon. I'm sure a bunch of missing bits will shake out as a result 🙃

@Timoses
Copy link

Timoses commented Nov 2, 2019

I really like the approach by Concourse CI to make "Resources" a container which handles 'get', 'put' and 'check' requests. It renders resources very modular, as anybody can write up a new image and use it as a resource.

Was there any consideration using a similar approach with TektonCD Resources? It would allow the community to create "resource containers" themselves and prevent in-tree implementations to support all the varieties (e.g. "I want Gitea support!" -> " Write your own resource container").

wlynch added a commit to wlynch/pipeline that referenced this issue Nov 4, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
wlynch added a commit to wlynch/pipeline that referenced this issue Nov 4, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
wlynch added a commit to wlynch/pipeline that referenced this issue Nov 5, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
wlynch added a commit to wlynch/pipeline that referenced this issue Nov 6, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
wlynch added a commit to wlynch/pipeline that referenced this issue Nov 6, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
wlynch added a commit to wlynch/pipeline that referenced this issue Nov 6, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
wlynch added a commit to wlynch/pipeline that referenced this issue Nov 8, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes tektoncd#1066.
tekton-robot pushed a commit that referenced this issue Nov 9, 2019
This changes the underlying implementation of the PullRequestResource
to jenkins-x/go-scm in order to take advantage of the repo drivers to
support other SCM providers.

In the process, this also allowed for refactoring to have a cleaner
distinction between interaction with SCM APIs and converting the data to
an on disk format.

BREAKING CHANGES:
* go-scm does not yet support updating comments, so that behavior is
removed for the time being since:
  1) it's unclear if anyone is actively
  using it.
  2) this behavior can be roughly replicated with a delete +
  create.
  3) the benefits of having the other SCM providers outweighs this
  feature.

Fixes #1066.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Issues or PRs that are questions around the project or a particular feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants