Migrate PullRequestResource to go-scm. #1521
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
the
size/XXL
|
Tests will not build until jenkins-x/go-scm#49 is submitted. |
|
/hold |
tekton-robot
added
the
do-not-merge/hold
cmd/pullrequest-init/api.go
Outdated
| logger *zap.SugaredLogger | ||
| } | ||
|
|
||
| // NewHandler initializes a new handler for interacting with GitHub |
There was a problem hiding this comment.
Thank you for this!
I'm still going through the review, one question I have is about comments, the commit message suggests they are not supported, however the PR includes code to handle them?
That was carried over from the previous implementation. Changed the name of the method to better describe what it does.
wlynch
force-pushed
the
go-scm
branch
3 times, most recently
from
51249f3
to
0e1c21e
Compare
|
/hold cancel All tests pass now, so removing the hold. @vdemeester if there was some other reason why you added the hold, feel free to reapply. =] |
tekton-robot
removed
the
do-not-merge/hold
I put the hold because of #1521 (comment) 👼 |
Perfect! That's now resolved. ^_^ |
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.
|
/lgtm |
|
Awesome! I'll take a detailed look tomorrow! |
1 similar comment
|
Awesome! I'll take a detailed look tomorrow! |
|
/lgtm Awesome! |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dlorenc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
This was accidentally removed in tektoncd#1521. We desperately need to figure out a story for running e2e tests that require auth secrets.
This was accidentally removed in #1521. We desperately need to figure out a story for running e2e tests that require auth secrets.
Changes
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:
removed for the time being since:
using it.
create.
feature.
Fixes #1066.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes