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.

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

Cleanup for PipelineRunStatus Migration after EmbeddedStatus Feature Flag is removed #6090

Closed
3 of 4 tasks
JeromeJu opened this issue Jan 31, 2023 · 5 comments · Fixed by #6099
Closed
3 of 4 tasks
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@JeromeJu
Copy link
Member

JeromeJu commented Jan 31, 2023

This issue tracks the cleanup after v0.45 post the EmbeddedStatus feature flag is removed.

  • Cleanup previous guards for status.taskruns or status.runs for full or both embedded-status
    - for example:
    // Verify that we don't populate child references for "full"
  • Remove status.taskruns and status.runs from the codebase
  • Refactor the helper functions that has been cut after the removal of the feature flag as they might be combined
  • Clean up the pkg/status package or not?

This was not cleaned up in v0.45 as now users might be in the progress of migrating off the embedded-status feature flag.

@tekton-robot tekton-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jan 31, 2023
@JeromeJu JeromeJu changed the title Cleanup for PipelineRunStatus after EmbeddedStatus Feature Flag is removed Cleanup for PipelineRunStatus Migration after EmbeddedStatus Feature Flag is removed Jan 31, 2023
@JeromeJu
Copy link
Member Author

JeromeJu commented Feb 1, 2023

/assign

@JeromeJu
Copy link
Member Author

JeromeJu commented Feb 1, 2023

I also noticed #4850 for changes in pkg/status, I think these should be kept specifically for those who wants to populate full/ both like PipelineRunStatus? So probably I should not even rename the functions nor the test cases in pkg/status when removing the Embedded-status?

cc @abayer could you help confirm my understanding on this, TY🙇‍♂️

@lbernick
Copy link
Member

lbernick commented Feb 2, 2023

When we remove the feature flag, we're no longer populating those fields, so it doesn't make sense to have the helpers continue to read from them

@JeromeJu
Copy link
Member Author

JeromeJu commented Feb 2, 2023

Thanks Lee! That makes sense, so shall we go ahead and delete the package pkg/status?

@lbernick
Copy link
Member

lbernick commented Feb 2, 2023

Let's leave the helpers there, but they should only read from childreferences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants