Skip to content

Refactors the mechanism for creating and updating TektonPipeline CR and TektonTrigger CR #628

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
Merged
tekton-robot merged 2 commits into from
Mar 1, 2022
Merged

Refactors the mechanism for creating and updating TektonPipeline CR and TektonTrigger CR #628

tekton-robot merged 2 commits into from
Mar 1, 2022

Conversation

@PuneetPunamiya
Copy link
Member

@PuneetPunamiya PuneetPunamiya commented Feb 20, 2022 •
edited
Loading

Changes

Refactoring the TektonPipeline CR

  • This patch now checks first if there is TektonPipeline CR
    already present, if yes then it checks if there are any
    changes in the instance, if yes then it makes the changes
    and updates the CR, if not then it creates a new CR

  • This patch also removes the polling state mechanism for
    tekton Pipeline CR

Refactoring the TektonTrigger CR

  • This patch now checks first if there is TektonTrigger CR
    already present, if yes then it checks if there are any
    changes in the instance, if yes then it makes the changes
    and updates the CR, if not then it creates a new CR

  • This patch also removes the polling state mechanism for
    tekton Trigger CR

Signed-off-by: Puneet Punamiya ppunamiy@redhat.com

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.

Release Notes

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Feb 20, 2022
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 20, 2022
@tekton-robot
Copy link
Contributor

tekton-robot commented Feb 20, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 48.3% -8.4

nikhil-thomas
nikhil-thomas reviewed Feb 21, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Feb 22, 2022
View reviewed changes
@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 22, 2022
@tekton-robot
Copy link
Contributor

tekton-robot commented Feb 22, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 67.7% 11.0

nikhil-thomas
nikhil-thomas reviewed Feb 22, 2022
View reviewed changes
@tekton-robot
Copy link
Contributor

tekton-robot commented Feb 22, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 66.7% 10.0

@PuneetPunamiya PuneetPunamiya changed the title [WIP] Updates the logic for creating and updating TektonPipeline CR Updates the logic for creating and updating TektonPipeline CR Feb 24, 2022
@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 Feb 24, 2022
@PuneetPunamiya
Copy link
Member Author

PuneetPunamiya commented Feb 24, 2022

/hold cancel

@nikhil-thomas
Copy link
Member

nikhil-thomas commented Feb 28, 2022

@PuneetPunamiya we also need similar changes for triggers in the code block following the the changes this patch makes.

Could you handle that also in this patch.

@nikhil-thomas
Copy link
Member

nikhil-thomas commented Feb 28, 2022

/approve

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2022
@PuneetPunamiya PuneetPunamiya changed the title Updates the logic for creating and updating TektonPipeline CR Refactors the mechanism for creating and updating TektonPipeline CR and TektonTrigger CR Mar 1, 2022
@tekton-robot
Copy link
Contributor

tekton-robot commented Mar 1, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 37.1% -19.6
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 71.2% 37.1% -34.1

@nikhil-thomas
Copy link
Member

nikhil-thomas commented Mar 1, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2022
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2022
@tekton-robot
Copy link
Contributor

tekton-robot commented Mar 1, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 37.1% -19.6
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 71.2% 37.1% -34.1

@tekton-robot
Copy link
Contributor

tekton-robot commented Mar 1, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 21.4% -35.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 71.2% 21.4% -49.8

@tekton-robot
Copy link
Contributor

tekton-robot commented Mar 1, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 37.1% -19.6
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 71.2% 37.1% -34.1

@PuneetPunamiya
Updates the logic for creating and updating TektonPipeline CR
8addbe6 
  - This patch now checks first if there is TektonPipeline CR
    already present, if yes then it checks if there are any
    changes in the instance, if yes then it makes the changes
    and updates the CR, if not then it creates a new CR

  - This patch also removes the polling state mechanism for
    tekton Pipeline CR

Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>
@PuneetPunamiya
Refactors the mechanism for creating and updating TektonTrigger CR
35f0dba 
   - This patch now checks first if there is TektonTrigger CR
     already present, if yes then it checks if there are any
     changes in the instance, if yes then it makes the changes
     and updates the CR, if not then it creates a new CR

   - This patch also removes the polling state mechanism for
     tekton Trigger CR

Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>
@tekton-robot
Copy link
Contributor

tekton-robot commented Mar 1, 2022

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.7% 37.1% -19.6
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 71.2% 37.1% -34.1

vdemeester
vdemeester approved these changes Mar 1, 2022
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2022
@tekton-robot
Copy link
Contributor

tekton-robot commented Mar 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhil-thomas, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [nikhil-thomas,vdemeester]

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 merged commit 9821997 into tektoncd:main Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdemeester vdemeester vdemeester approved these changes

@concaf concaf Awaiting requested review from concaf

@piyush-garg piyush-garg Awaiting requested review from piyush-garg

+1 more reviewer

@nikhil-thomas nikhil-thomas nikhil-thomas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@vdemeester vdemeester

@nikhil-thomas nikhil-thomas

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PuneetPunamiya @tekton-robot @nikhil-thomas @vdemeester