-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
feat: Add @GQLExcluded #1141
feat: Add @GQLExcluded #1141
Conversation
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.
Looks good, just some minor comments.
@@ -64,6 +64,7 @@ trait SchemaDerivation[R] extends LowPriorityDerivedSchema { | |||
Some(getName(ctx)), | |||
getDescription(ctx), | |||
ctx.parameters | |||
.filter(p => !p.annotations.exists(_ == GQLExcluded())) |
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.
How about .filterNot(_.annotations.exists(_ == GQLExcluded()))
?
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.
👍 didn't know that existed!
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.
Don't like p => p
=> _
? 😆
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.
Haha sure! I sometimes try to shy away from multiple levels of _
but I think it's fine in this case!
Can you also add it here: https://github.com/ghostdogpr/caliban/blob/master/vuepress/docs/docs/schema.md#annotations But a separate PR would be great, so that I can merge the doc PR after the next release. That way it doesn't appear on the website before it's released (docs are updated right after they're merged to master). |
Absolutely, here's a separate PR: #1142 |
Closes #1040.
Didn't add support to exclude an enum member since it felt like it'd need special handling (what are we supposed to output as result if we were to get back a value that's
GQLExcluded
?).