Skip to content

Improving exception for duplicated field definition#714

Merged
andimarek merged 2 commits into
graphql-java:masterfrom
jdorleans:hotfix/improve-field-definition-exception
Sep 17, 2017
Merged

Improving exception for duplicated field definition#714
andimarek merged 2 commits into
graphql-java:masterfrom
jdorleans:hotfix/improve-field-definition-exception

Conversation

@jdorleans
Copy link
Copy Markdown
Contributor

At first, I found difficult to understand what the exception meant and where to find the problem to fix. So, I'm providing clearer description for that. Supposing the following schema:

type User {
  ...
  password: String
  password: String  // definition duplicated
}

We'll get the following exception:

Current:
Exception in thread "main" graphql.AssertException: field password redefined

Update:
Exception in thread "main" graphql.AssertException: Duplicated definition for field 'password' in type 'User'

Copy link
Copy Markdown
Member

@andimarek andimarek left a comment

Choose a reason for hiding this comment

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

Hi,

thanks for your PR. Looks goods except the removed empty javadoc lines.

Thank you!

* from within
*
* @param builder an un-built/incomplete GraphQLFieldDefinition
*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you please not remove this extra line?

* </pre>
*
* @param builderFunction a supplier for the builder impl
*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you please not remove this extra line?

@jdorleans
Copy link
Copy Markdown
Contributor Author

No worries, I can add them back. I just followed the other files format which seems to be aligned with java standards better.

@andimarek
Copy link
Copy Markdown
Member

thanks

@andimarek andimarek merged commit 4f1a30d into graphql-java:master Sep 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants