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

Add type meta data for list types #1380

Merged
merged 3 commits into from
Jan 3, 2024
Merged

Conversation

Danil-Grigorev
Copy link
Member

Motivation

While implementing generic resource listing from discovery results, it seems the type meta is omitted, though it is guarantied to be supplied on the list resource type. This makes it hard to know the resource kind if the ApiCapabilities goes out of scope.

Solution

Store the TypeMeta with the resource list result.

Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
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 for this. Generally looks sensible, just a few nits on tests and an optional suggestion for TypeMeta

kube-core/src/object.rs Show resolved Hide resolved
kube-core/src/object.rs Outdated Show resolved Hide resolved
kube-core/src/object.rs Show resolved Hide resolved
@clux clux added the changelog-change changelog change category for prs label Dec 27, 2023
@clux clux added this to the 0.88.0 milestone Dec 27, 2023
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.

last nit on naming, new ctors look great and have great docs. ty!

kube-core/src/metadata.rs Outdated Show resolved Hide resolved
kube-core/src/object.rs Show resolved Hide resolved
Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
Copy link

codecov bot commented Dec 30, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (00fffed) 72.1% compared to head (cc20027) 72.1%.
Report is 1 commits behind head on main.

❗ Current head cc20027 differs from pull request most recent head 4e76988. Consider uploading reports for the commit 4e76988 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1380     +/-   ##
=======================================
- Coverage   72.1%   72.1%   -0.0%     
=======================================
  Files         75      75             
  Lines       6435    6459     +24     
=======================================
+ Hits        4635    4652     +17     
- Misses      1800    1807      +7     
Files Coverage Δ
kube-core/src/object.rs 84.2% <100.0%> (+4.8%) ⬆️
kube/src/mock_tests.rs 97.9% <ø> (ø)
kube-core/src/metadata.rs 67.7% <0.0%> (-14.4%) ⬇️

... and 2 files with indirect coverage changes

@clux clux mentioned this pull request Jan 3, 2024
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 again for this. Have fixed up the MSRV build so this should be mergeable now.

@clux clux enabled auto-merge (squash) January 3, 2024 07:35
@clux clux merged commit f2090b8 into kube-rs:main Jan 3, 2024
15 of 16 checks passed
@clux clux mentioned this pull request Jan 3, 2024
@clux clux added changelog-add changelog added category for prs and removed changelog-change changelog change category for prs labels Jan 3, 2024
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.

2 participants