-
Notifications
You must be signed in to change notification settings - Fork 110
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
Remove the expand
option on the describe
route
#1755
Conversation
@@ -222,7 +222,7 @@ impl<'a> EntityDef<'a> { | |||
} | |||
} | |||
|
|||
fn entity_description_json(description: WithTypespace<EntityDef>, expand: bool) -> Option<Value> { | |||
fn entity_description_json(description: WithTypespace<EntityDef>) -> Option<Value> { |
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.
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.
Seems fine to me
126acd3
to
4d1cb43
Compare
if module_def && expand.is_some() { | ||
return Err(( | ||
StatusCode::BAD_REQUEST, | ||
"expand and module_def cannot both be specified", |
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.
@coolreader18 why was this constraint here? Does it invalidate the returned thing somehow? (eg something about the typespace?)
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.
Just because they do opposite things - expand makes the hacky representation more detailed, while module_def uses a non-hacky representation.
…nto bfops/remove-expand
Description of Changes
The
expand
option on thedescribe
route didn't do much - it just omitted theschema
field from the result.This PR removes it, and the corresponding
--brief
/-b
flag on the CLI subcommand.API and ABI breaking changes
Yes, we're removing some functionality. As far as we know, nobody relies on this.
Expected complexity level and risk
2
Testing
Smoketests still passing