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

replace derivative dependency with educe #1585

Merged
merged 1 commit into from
Sep 22, 2024

Conversation

rorosen
Copy link
Contributor

@rorosen rorosen commented Sep 18, 2024

Replace the unmaintained derivative dependency with educe. See also #1583.

Motivation

The derivative dependency is unmaintained and uses syn 1 while everything else uses syn 2.

Solution

I decided to replace derivative with educe instead of derive-more, as derive-more lacks support of skipping/ignoring fields (see #311) and cannot derive Eq and PartialEq. However, I haven't tried too hard to make it work so let me know if I should try to use derive-more anyway.

Although educe can almost be used as a drop-in replacement in most places, it lacks to specify custom bounds for Eq for some reason. I decided to write the manageable impl for Eq myself where needed, i.e. in mod.rs and object_ref.rs

Edit: Noticed by CI checks that educe triggers clippy::unused_underscore_binding when deriving Debug for enums (i.e. for State<K> in watcher.rs). Hence, I added #![allow(clippy::used_underscore_binding)] in kube-runtime, not sure whether that is acceptable...

Copy link

codecov bot commented Sep 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.3%. Comparing base (edd384e) to head (b57cf5d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1585   +/-   ##
=====================================
  Coverage   75.3%   75.3%           
=====================================
  Files         82      82           
  Lines       7331    7331           
=====================================
  Hits        5519    5519           
  Misses      1812    1812           
Files with missing lines Coverage Δ
kube-runtime/src/controller/mod.rs 29.9% <ø> (ø)
kube-runtime/src/reflector/dispatcher.rs 96.3% <ø> (ø)
kube-runtime/src/reflector/object_ref.rs 69.4% <ø> (ø)
kube-runtime/src/reflector/store.rs 96.9% <ø> (ø)
kube-runtime/src/scheduler.rs 94.8% <ø> (ø)
kube-runtime/src/utils/delayed_init.rs 97.1% <ø> (ø)
kube-runtime/src/watcher.rs 44.2% <ø> (ø)

Replace the unmaintained `derivative` dependency with `educe`.
See also kube-rs#1583.

Signed-off-by: Robert Rose <robert.rose@mailbox.org>
Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

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

Thanks a lot for doing this. Finally had the chance to look over this. Was worried about Eq, but it seems all good to me!

kube-runtime/src/controller/mod.rs Show resolved Hide resolved
kube-runtime/src/reflector/object_ref.rs Show resolved Hide resolved
deny.toml Show resolved Hide resolved
kube-runtime/src/lib.rs Show resolved Hide resolved
@clux
Copy link
Member

clux commented Sep 21, 2024

Marking as a fix as I don't think this will need to be classified as a breaking change. Though, likely one for the next minor anyway.

@clux clux added the changelog-fix changelog fix category for prs label Sep 21, 2024
@clux clux added this to the 0.96.0 milestone Sep 21, 2024
@clux clux added changelog-remove changelog remove category for prs changelog-exclude changelog excluded prs changelog-fix changelog fix category for prs and removed changelog-fix changelog fix category for prs changelog-remove changelog remove category for prs changelog-exclude changelog excluded prs labels Sep 21, 2024
@clux clux linked an issue Sep 21, 2024 that may be closed by this pull request
@clux clux merged commit c0fe0c8 into kube-rs:main Sep 22, 2024
17 checks passed
@rorosen rorosen deleted the replace-derivative branch September 23, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-fix changelog fix category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replace derivative dependency
3 participants