-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix(plugins): rename NoHelmChartTestFailuresCondition to HelmChartTestSucceededCondition #748
Conversation
…tSucceededCondition Signed-off-by: Akshay Iyyadurai Balasundaram <[email protected]>
…Condition for clarity Signed-off-by: Akshay Iyyadurai Balasundaram <[email protected]>
…' into improve-plugin-test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will add the new HelmChartTestSucceeded
condition to the status of a Plugin. However, the existing condition will not be removed. To avoid manual patching of the existing Plugins, I would suggest removing the old condition via the controller first. Once this is rolled out, the logic can be removed again.
Signed-off-by: Akshay Iyyadurai Balasundaram <[email protected]>
…' into improve-plugin-test
…if exist Signed-off-by: Akshay Iyyadurai Balasundaram <[email protected]>
…' into improve-plugin-test
…ry slices.DeleteFunc Signed-off-by: Akshay Iyyadurai Balasundaram <[email protected]>
…' into improve-plugin-test
This is a great suggestion. I have done as you have suggested and I have also added some tests :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tracked the removal of the deprecated Condition here: #754
@IvoGoman Why does the CI and other GHA workflows have not run for this PR? |
Ah I see.. |
There is still a linter issue open: https://github.com/cloudoperators/greenhouse/actions/runs/11897250768/job/33189959724#step:4:205 |
You can see the status check for the commit before CRD API commit where the checks are failing. |
Signed-off-by: Akshay Iyyadurai Balasundaram <[email protected]>
The current status condition
NoHelmChartTestFailures
uses a double negative that makes it difficult to understand at a glance. Based on user feedback, we're renaming it to be more intuitive and direct.This change makes the status condition:
The functionality remains the same - the condition indicates whether the Helm chart tests is executed successfully.
Signed-off-by: Akshay Iyyadurai Balasundaram [email protected]
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
What type of PR is this? (check all applicable)
Related Tickets & Documents
Added tests?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added to documentation?
Checklist