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

Hoist enum values from subschemas #1051

Merged
merged 4 commits into from Oct 17, 2022
Merged

Conversation

nightkr
Copy link
Member

@nightkr nightkr commented Oct 14, 2022

Fixes #1047

Currently this drops variant-level documentation, do we want to include it in the enum property's documentation instead?

Fixes kube-rs#1047

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>
Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>
@nightkr nightkr self-assigned this Oct 14, 2022
@nightkr nightkr requested a review from a team October 14, 2022 09:50
@nightkr nightkr added bug Something isn't working changelog-fix changelog fix category for prs core generic apimachinery style work labels Oct 14, 2022
@nightkr nightkr added this to the 0.76.0 milestone Oct 14, 2022
Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>
Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

👏🏼 Used this patch off of my code mentioned in #1047 and it solves my usecase - thanks!

"type": "string",
"enum": ["Female", "Male", "Other"],
"description": "Sex of the person"
"description": "Gender of the person"
Copy link
Member

Choose a reason for hiding this comment

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

Currently this drops variant-level documentation, do we want to include it in the enum property's documentation instead?

Do you mean something like this?

"description": "Gender of the person\n\nOther - This variant has a comment!"

I think it's better to keep the current version because users can still add variant-level documentation manually if they'd like. Automatically including them will be a nice opt-in feature, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, basically.

@codecov-commenter
Copy link

Codecov Report

Merging #1051 (9dd40ab) into main (9d1a554) will increase coverage by 0.07%.
The diff coverage is 96.42%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1051      +/-   ##
==========================================
+ Coverage   72.24%   72.32%   +0.07%     
==========================================
  Files          65       65              
  Lines        4687     4704      +17     
==========================================
+ Hits         3386     3402      +16     
- Misses       1301     1302       +1     
Impacted Files Coverage Δ
kube-core/src/schema.rs 89.70% <95.83%> (+1.47%) ⬆️
kube-derive/tests/crd_schema_test.rs 100.00% <100.00%> (ø)

@nightkr nightkr merged commit da6b5e7 into kube-rs:main Oct 17, 2022
zen-xu added a commit to zen-xu/habitat that referenced this pull request Nov 2, 2022
Since kube fix it in kube-rs/kube#1051, we can use doc comment on variants
zen-xu added a commit to zen-xu/habitat that referenced this pull request Nov 2, 2022
Since kube fix it in kube-rs/kube#1051, we can use doc comment on variants
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working changelog-fix changelog fix category for prs core generic apimachinery style work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression using schemars 0.8.11 with CRDs openAPIV3Schema that use enums
4 participants