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

Remove abandoned StreamSubscribe implementation #1470

Merged
merged 3 commits into from May 1, 2024
Merged

Conversation

clux
Copy link
Member

@clux clux commented Apr 18, 2024

This was never properly usable without further integration, and as such was never stabilised. It is replaced via #1449

..alternatively we could deprecate this for a few releases, doesn't hurt us either way, nothing relies on this.

This was never properly usable without further integration, and as such
was never stabilised. It is replaced via #1449

Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux clux added the changelog-remove changelog remove category for prs label Apr 18, 2024
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.4%. Comparing base (9eb5048) to head (048d476).

❗ Current head 048d476 differs from pull request most recent head b16f74f. Consider uploading reports for the commit b16f74f to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1470     +/-   ##
=======================================
- Coverage   74.6%   74.4%   -0.2%     
=======================================
  Files         79      78      -1     
  Lines       6871    6772     -99     
=======================================
- Hits        5125    5034     -91     
+ Misses      1746    1738      -8     
Files Coverage Δ
kube-runtime/src/utils/mod.rs 62.3% <ø> (ø)
kube-runtime/src/utils/watch_ext.rs 22.3% <ø> (+1.2%) ⬆️

... and 4 files with indirect coverage changes

@clux clux added the unstable feature that unstable feature gating label Apr 18, 2024
@clux clux added this to the 0.91.0 milestone Apr 26, 2024
@clux clux marked this pull request as ready for review April 26, 2024 19:35
Copy link
Contributor

@mateiidavid mateiidavid left a comment

Choose a reason for hiding this comment

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

Late on reviewing. fwiw I pondered a bit during these past few days on whether removing it outright is better than marking it as deprecated for a release or two. My main concern was that this is behind the same unstable flag as the newer API.

We can't really use this for anything though. It can't be threaded through a controller. Makes sense to me to just remove it.

@clux clux enabled auto-merge (squash) May 1, 2024 19:26
@clux clux merged commit 6d0c9b1 into main May 1, 2024
17 checks passed
@clux clux deleted the rm-depr-stream-subscribe branch May 1, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-remove changelog remove category for prs unstable feature that unstable feature gating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants