Skip to content

Add objective query#11

Merged
lucca65 merged 3 commits into
masterfrom
feature/objective-query
Nov 1, 2019
Merged

Add objective query#11
lucca65 merged 3 commits into
masterfrom
feature/objective-query

Conversation

@lucca65
Copy link
Copy Markdown
Member

@lucca65 lucca65 commented Oct 22, 2019

No description provided.

@lucca65 lucca65 requested a review from zacck-zz October 22, 2019 03:11
Copy link
Copy Markdown
Contributor

@zacck-zz zacck-zz left a comment

Choose a reason for hiding this comment

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

@lucca65 looks good but missing unit tests for regression coverage, I also moved the inputs to input objects to make it easier to use and uniform

@lucca65
Copy link
Copy Markdown
Member Author

lucca65 commented Oct 22, 2019

Hey @zacck I'll do the tests.

Can you explain to me why is the input objects important? For me they only make the query more complex and I can't see any benefits for using it. For filters or more complex inputs it is completely fine, but even in the GraphQL project home page they don't use it:

Screen Shot 2019-10-22 at 14 53 27

@zacck-zz
Copy link
Copy Markdown
Contributor

zacck-zz commented Oct 23, 2019

@lucca65 I wouldn't expect them to show them in the Graphql homepage since, it adds to the learning curve. In any case input objects introduce ease of auto generation of code as we do with the elm-graphql library and as we go along and as our inputs grow its easier to modify an input object rather than building up complex arguments.

It's one of those things I have picked up working with multiple graphql APIs let me know if I can send you more texts to justify if you need ...

@lucca65 lucca65 merged commit 11813d1 into master Nov 1, 2019
@lucca65 lucca65 deleted the feature/objective-query branch November 1, 2019 15:35
lucca65 added a commit that referenced this pull request May 17, 2026
* Add objective query
* Use input objects for uniformity and ease
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.

3 participants