refactoring - pipelinerunstate #3231
Conversation
tekton-robot
added
release-note-none
|
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
thank you @pritidesai!
i suggest we update the instances where pipelinerunstate is referred to as pipelinestate
Creating a new file with all attributes of pipelinerunstate along with the test file. Refactored few functions as an attributes of pipelunerunstate
pritidesai
force-pushed
the
pipelinerunstate
branch
from
26211bf
to
6dd76dc
Compare
|
The following is the coverage report on the affected files.
|
|
/lgtm |
There was a problem hiding this comment.
Looks like this is pretty much a "pure" refactoring, i.e. no logic changes at all, so looks great to me and i like the incremental step toward improvement.
/approve
We can create a new package and move it under that package in a separate PR.
I think that's a great next step! It would be interesting to maybe even step back and take a look at what end state we'd like to get to
| return skipped | ||
| } | ||
|
|
||
| func (state PipelineRunState) GetTaskRunsStatus(pr *v1beta1.PipelineRun) map[string]*v1beta1.PipelineRunTaskRunStatus { |
There was a problem hiding this comment.
nice - i like that none of these have pointer receivers so we don't have to worry about side effects :D
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bobcatfish 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
Changes
(restructuring only - no updates to any of the functions)
Creating a new file with all attributes of
pipelinerunstatealong with the test file (moved unit tests frompipelinerunresolution_test.gotopipelinerunstate_test.go)Refactored following functions as an attributes of
pipelunerunstate:I have left all of the functions under the same package
resourcesfor simplicity. We can create a new package and move it under that package in a separate PR./kind cleanup
/cc @bobcatfish @jerop
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