-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Enum key/value description #952
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #952 +/- ##
==========================================
+ Coverage 98.61% 98.63% +0.02%
==========================================
Files 68 68
Lines 8074 8126 +52
==========================================
+ Hits 7962 8015 +53
+ Misses 112 111 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Thanks to @valentijnscholten for the first attempt in #403
return append_meta(build_choice_field(field), meta) | ||
schema = build_choice_field(field) | ||
if 'description' in meta: | ||
meta['description'] = meta['description'] + '\n\n' + schema.pop('description') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generates a KeyError
if schema
does not have description
. You need to use schema.pop('description', None)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tfranzel this is breaking our current build, because we set "ENUM_GENERATE_CHOICE_DESCRIPTION": False
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, can't seem do a release without a .1
apparently.
You need to use schema.pop('description', None)
would lead to a None str concat TypeError
, but I get the point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will wait for the release until tonight to see whether something else will come up.
if prop_schema.get('description', '').startswith('*'): | ||
enum_schema['description'] = prop_schema.pop('description') | ||
elif '\n\n*' in prop_schema.get('description', ''): | ||
_, _, post = prop_schema['description'].partition('\n\n*') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apology for commenting on merging MR, @tfranzel .
Just a question about these changes...
We have a serializer with field (choices) and it has help_text
. Unfortunately this help text is not preserved in the description of the enum due to this line. How we can preserve this description along with choices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sshishov, the help_text
should still be there in the serializer component (NOT the enum component as it belongs to the field). This could otherwise lead confusing descriptions actually belonging somewhere else.
If that is not the case, please open new issue with a reproduction and full example. thx
add optional (default on) feature to show enum key/value listing in description string like outlined in #403.
related issues: #337 #403 #105 #563