Skip to content

Conversation

vdemeester
Copy link
Member

Changes

This adds

  • auto-conversion methods and tests for TaskRun
  • auto-conversion methods and tests for PipelineRun

Depends on #2002
/hold

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.

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 10, 2020
@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Feb 10, 2020
@tekton-robot tekton-robot requested review from a user and dibyom February 10, 2020 10:46
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 10, 2020
@vdemeester
Copy link
Member Author

/cc @bobcatfish

@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/v1alpha2/cluster_task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipeline_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipelinerun_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/taskrun_conversion.go Do not exist 100.0%

@vdemeester vdemeester force-pushed the v1alpha2-auto-conversion-run branch from 7c083dd to 5c387d0 Compare February 10, 2020 11:25
@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/v1alpha2/cluster_task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipeline_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipelinerun_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/taskrun_conversion.go Do not exist 100.0%

@vdemeester vdemeester force-pushed the v1alpha2-auto-conversion-run branch from 5c387d0 to 843ac21 Compare February 10, 2020 11:59
@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/v1alpha2/cluster_task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipeline_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipelinerun_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/taskrun_conversion.go Do not exist 100.0%


t := &v1alpha1.TaskSpec{
Steps: []v1alpha1.Step{rcc.Condition.Spec.Check},
TaskSpec: v1alpha2.TaskSpec{
Copy link

Choose a reason for hiding this comment

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

nit: in a bunch of other places we're not nesting this on a newline. We're doing t := &v1alpha1.TaskSpec{TaskSpec: v1alpha2.TaskSpec{)

I really don't mind either way which format is used but figure we should probably be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Usually I am doing &v1alpha1.TaskSpec{TaskSpec: v1alpha2.TaskSpec{ if TaskSpec is the only field in the struct, otherwise I aligne them 😅

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 10, 2020
@vdemeester vdemeester force-pushed the v1alpha2-auto-conversion-run branch from 843ac21 to 6a3198b Compare February 11, 2020 09:33
@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/v1alpha2/cluster_task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipeline_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/pipelinerun_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/task_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/taskrun_conversion.go Do not exist 100.0%

@vdemeester vdemeester force-pushed the v1alpha2-auto-conversion-run branch from 6a3198b to daf5ded Compare February 11, 2020 10:25
@vdemeester
Copy link
Member Author

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 11, 2020
@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/v1alpha2/pipelinerun_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/taskrun_conversion.go Do not exist 100.0%

This adds auto-conversion methods and tests for TaskRun types in
v1alpha1 and v1alpha2.

Signed-off-by: Vincent Demeester <[email protected]>
This adds auto-conversion methods and tests for PipelineRun types in
v1alpha1 and v1alpha2.

Signed-off-by: Vincent Demeester <[email protected]>
@vdemeester vdemeester force-pushed the v1alpha2-auto-conversion-run branch from daf5ded to 3e65f90 Compare February 11, 2020 10:29
@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/v1alpha2/pipelinerun_conversion.go Do not exist 100.0%
pkg/apis/pipeline/v1alpha2/taskrun_conversion.go Do not exist 100.0%

@vdemeester vdemeester mentioned this pull request Feb 11, 2020
8 tasks
@chmouel
Copy link
Member

chmouel commented Feb 12, 2020

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 12, 2020
@vdemeester
Copy link
Member Author

/test pull-tekton-pipeline-build-tests

@tekton-robot tekton-robot merged commit 1266b4a into tektoncd:master Feb 12, 2020
@vdemeester vdemeester deleted the v1alpha2-auto-conversion-run branch February 12, 2020 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

4 participants