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.

Sign up for GitHub

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

Jump to bottom

Add gitlab support to the PR resource. #1550

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Add gitlab support to the PR resource. #1550

merged 1 commit into from
Dec 3, 2019

Conversation

dlorenc
Copy link
Contributor

@dlorenc dlorenc commented Nov 11, 2019

Changes

This refactors the PR init container a bit to add support for multiple SCM providers.
This PR also adds support for gitlab, as the second supported SCM provider.

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:

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

Add support for Gitlab in the PR resource.

@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 11, 2019
@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Nov 11, 2019
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 11, 2019
@dlorenc
Copy link
Contributor Author

dlorenc commented Nov 11, 2019

/test pull-tekton-pipeline-integration-tests

wlynch
wlynch reviewed Nov 11, 2019
View reviewed changes
pkg/pullrequest/scm.go Outdated Show resolved Hide resolved
pkg/pullrequest/scm.go Outdated Show resolved Hide resolved
@dlorenc dlorenc force-pushed the gitlab2 branch 2 times, most recently from ecd785c to aa539f0 Compare November 11, 2019 18:02
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@dlorenc
Copy link
Contributor Author

dlorenc commented Nov 11, 2019

Looks like jenkins-x/go-scm is missing a few things still. Hold on a bit...

@dlorenc
Copy link
Contributor Author

dlorenc commented Nov 12, 2019

I need to get jenkins-x/go-scm#54 in first, then update.

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@dlorenc dlorenc force-pushed the gitlab2 branch 2 times, most recently from a8ae01e to ef0256a Compare November 18, 2019 19:23
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@dlorenc
Copy link
Contributor Author

dlorenc commented Nov 19, 2019

/test pull-tekton-pipeline-integration-tests

@dlorenc dlorenc changed the title WIP: Add gitlab support to the PR resource. Add gitlab support to the PR resource. Nov 19, 2019
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2019
@tekton-robot tekton-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 26, 2019
wlynch
wlynch reviewed Nov 26, 2019
View reviewed changes
cmd/pullrequest-init/main.go Outdated Show resolved Hide resolved
pkg/apis/pipeline/v1alpha1/pull_request_resource.go Outdated Show resolved Hide resolved
wlynch
wlynch reviewed Nov 27, 2019
View reviewed changes
pkg/pullrequest/api.go Outdated Show resolved Hide resolved
@wlynch wlynch mentioned this pull request Nov 27, 2019
Closed
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2019
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

wlynch
wlynch reviewed Dec 3, 2019
View reviewed changes
Copy link
Member

@wlynch wlynch 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! Just a few more minor comments.

docs/resources.md Outdated Show resolved Hide resolved
pkg/pullrequest/api.go Outdated Show resolved Hide resolved
pkg/pullrequest/scm.go Outdated Show resolved Hide resolved
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@dlorenc
Add gitlab support to the PR resource.
7978c12 
This refactors the PR init container a bit to add support for multiple SCM providers.
This PR also adds support for gitlab, as the second supported SCM provider. To do this,
we needed a few other changes:
- Switch from CombinedStatus to Statuses
- Switch from the Issues service to the PullRequests service for labels and comments. These
are equivalent for Github, but not Gitlan and other providers.
- Update go-scm
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pull_request_resource.go 85.0% 87.5% 2.5

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ImJasonH, wlynch

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wlynch
Copy link
Member

wlynch commented Dec 3, 2019

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2019
@tekton-robot tekton-robot merged commit d478e7d into tektoncd:master Dec 3, 2019
@bobcatfish bobcatfish mentioned this pull request Jan 7, 2020
Closed
bobcatfish
bobcatfish reviewed Jan 7, 2020
View reviewed changes
docs/resources.md
@@ -422,7 +422,7 @@ spec:
- name: url
value: https://github.com/wizzbangcorp/wizzbang/pulls/1
secrets:
- fieldName: githubToken
- fieldName: authToken
Copy link
Collaborator

@bobcatfish bobcatfish Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

hm this was a backwards incompatible change 🤔 we made some other backwards incompatible changes to the pull request resource around the same time (e.g. the format of files like status), would have been great to roll these out in a backwards compatible way first

(this turned out to be the root of my sorrow in #1777 😭 )

@imjasonh @wlynch

@bobcatfish bobcatfish mentioned this pull request Jan 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobcatfish bobcatfish bobcatfish left review comments

@wlynch wlynch wlynch approved these changes

@abayer abayer Awaiting requested review from abayer

@vdemeester vdemeester Awaiting requested review from vdemeester

Assignees

@imjasonh imjasonh

@wlynch wlynch

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dlorenc @tekton-robot @imjasonh @wlynch @bobcatfish @googlebot