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

chore: remove latest version for conversion webhook #760

Conversation

MathieuCesbron
Copy link
Contributor

Description

The point of the conversion webhook is too update some v1beta1 fields basically and it does nothing for v1beta2. I removed the conversion webhook for v1beta2 version because there is no need to call the conversion for them.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Tests have been added/modified and all tests pass.
  • Functionality/bugs have been confirmed to be unchanged or fixed.
  • I have performed a self-review of my own code.
  • Documentation has been updated or added where necessary.

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3e6fcb9) 32.79% compared to head (bbb43fc) 32.79%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #760   +/-   ##
=======================================
  Coverage   32.79%   32.79%           
=======================================
  Files          19       19           
  Lines        3232     3232           
=======================================
  Hits         1060     1060           
  Misses       2113     2113           
  Partials       59       59           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MathieuCesbron MathieuCesbron marked this pull request as ready for review January 25, 2024 17:56
@drivebyer
Copy link
Collaborator

Some people may still use v1beta1. IMO, we still need conversion in a period of time.

@MathieuCesbron
Copy link
Contributor Author

Thanks for the comment. Yes, I agree we should only trigger the conversion for v1beta1 version and not for v1beta2 because it’s gonna convert nothing.

That’s why I removed it from the selector. Am I missing something ?

@drivebyer
Copy link
Collaborator

I'm not familiar with the conversionReviewVersions mechanism; maybe need help.@shubham-cmyk

@drivebyer drivebyer changed the title Remove latest version for conversion webhook chore: remove latest version for conversion webhook Jun 21, 2024
@drivebyer drivebyer merged commit dd4e8e1 into OT-CONTAINER-KIT:master Jun 21, 2024
29 of 30 checks passed
mattrobinsonsre pushed a commit to mattrobinsonsre/redis-operator that referenced this pull request Jul 11, 2024
…#760)

Remove latest version for conversion webhook

Signed-off-by: Mathieu Cesbron <[email protected]>
Signed-off-by: Matt Robinson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants