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

Derive PartialEq on core params structs #1237

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Conversation

danrspencer
Copy link
Contributor

Motivation

We use the command pattern to pass Kubernetes commands through our reconciliation code. We want to be able to compare those commands in unit tests to ensure we're getting out the expected commands. Currently we have to new type the params structs.

Solution

Added PartialEq to the params structs.

Signed-off-by: Dan Spencer <danrspen@gmail.com>
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #1237 (7bcae99) into main (9dd7bf5) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1237   +/-   ##
=======================================
  Coverage   71.93%   71.93%           
=======================================
  Files          71       71           
  Lines        5725     5725           
=======================================
  Hits         4118     4118           
  Misses       1607     1607           
Impacted Files Coverage Δ
kube-core/src/params.rs 80.69% <ø> (ø)

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, LGTM!

@clux clux added the changelog-add changelog added category for prs label Jul 3, 2023
@clux clux added this to the 0.84.0 milestone Jul 3, 2023
@clux clux changed the title Added PartialEq to the params structs Derive PartialEq to the core params structs Jul 3, 2023
@Dav1dde
Copy link
Member

Dav1dde commented Jul 3, 2023

Wonder if deriving Eq would also be okay/a good idea?

@clux clux changed the title Derive PartialEq to the core params structs Derive PartialEq on core params structs Jul 3, 2023
@clux
Copy link
Member

clux commented Jul 4, 2023

Both are probably fine, but we are generally lacking Eq in the library throughout. For consistency, will merge this as is, but we can always discuss whether it's worth generating Eq on most of our stuff. Am unsure atm.

@clux clux merged commit bd14267 into kube-rs:main Jul 4, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-add changelog added category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants