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

Allow to rename, describe and set default arguments using DeriveObjectSettings. #339

Merged
merged 4 commits into from
Feb 18, 2018

Conversation

fehu
Copy link
Contributor

@fehu fehu commented Feb 12, 2018

#318: Added the following DeriveObjectSettings:

Setting name Description
MethodArgumentRename change name of single argument
MethodArgumentDescription set description for a single argument
MethodArgumentsDescription set descriptions for multiple arguments
MethodArgumentDefault set default value for a single argument
MethodArgument set description and default value for a single argument

Copy link
Member

@OlegIlyenko OlegIlyenko left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on this feature! It looks great 👍

build.sbt Outdated
@@ -1,6 +1,6 @@
name := "sangria"
organization := "org.sangria-graphql"
version := "1.3.3"
version := "1.3.4-SNAPSHOT"
Copy link
Member

Choose a reason for hiding this comment

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

👍 I missed this one :)

getArgument(pos, methodName, argName).right.map{
case arg if argType <:< arg.typeSignature.resultType ⇒ Nil
case arg ⇒ List(
pos → s"Wrong type of default value '$default' for argument '$argName' of method '$methodName': expected '${arg.typeSignature.resultType}', but got '$argType'."
Copy link
Member

Choose a reason for hiding this comment

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

I think we would need to rely on the standard set of validations for a default value type. It is based on ArgumentType, ToInput type-classes as well as some run-time validations. It still will provide the same compile-time guarantees. It's a minor thing, I will adjust it after the merge.

@OlegIlyenko OlegIlyenko merged commit 8a923ac into sangria-graphql:master Feb 18, 2018
@OlegIlyenko OlegIlyenko added this to the v1.4.0 milestone Feb 18, 2018
@fehu
Copy link
Contributor Author

fehu commented Feb 19, 2018

Thank you for accepting my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants