Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up1210 deferred aligned with apollo #1221
Conversation
This reverts commit 38dfab1
|
see my comments .... @bbakerman great work! |
|
|
||
| import java.util.List; | ||
|
|
||
| public abstract class DeferredDirectiveAbstractRule extends AbstractRule { |
andimarek
Sep 15, 2018
Author
Member
should be Internal
should be Internal
|
|
||
| import java.util.List; | ||
|
|
||
| public class DeferredDirectiveOnNonNullableField extends DeferredDirectiveAbstractRule { |
andimarek
Sep 15, 2018
Author
Member
should be Internal
should be Internal
...ain/java/graphql/validation/rules/DeferredDirectiveOnQueryOperation.java
Show resolved
Hide resolved
src/main/java/graphql/validation/rules/DeferredMustBeOnAllFields.java
Outdated
Show resolved
Hide resolved
| Directive directive = field.getDirective(Directives.DeferDirective.getName()); | ||
| if (directive != null) { | ||
| Map<String, Object> argumentValues = valuesResolver.getArgumentValues(Directives.DeferDirective.getArguments(), directive.getArguments(), variables); | ||
| return (Boolean) argumentValues.get("if"); |
andimarek
Sep 15, 2018
Author
Member
can this be null? => unboxing will cause NPE
can this be null? => unboxing will cause NPE
bbakerman
Mar 15, 2019
Member
no - its default is true
no - its default is true
bbakerman
Mar 15, 2019
Member
now made it non null as an arg declaration
now made it non null as an arg declaration
| echo "Building on master" | ||
| BUILD_COMMAND="./gradlew clean assemble && ./gradlew check --info && ./gradlew bintrayUpload -x check --info" | ||
| fi | ||
|
|
andimarek
Sep 15, 2018
Author
Member
we must remember that must be changed again before merge!
we must remember that must be changed again before merge!
bbakerman
Mar 15, 2019
Member
you did that
you did that
fix SubscriberPublisher test: should throw NPE
|
I'm wondering what the status of this branch is? It's very important for my project (and seems to work well). Would you like help cleaning/documenting to get it merged? Just point me in the right direction |
|
@bbakerman your opinion on the current state of it? |
|
the implementation here looks good, but it is not fully tested and we will not merge it soon. |
|
There is a valuable feature because current deferred has a problem with matching paths. So, is any forecasts of that feature? We want to use it, and here is the question: wait for this PR will be merged or use this branch or maybe do the fork? Do the tests is the main problem with that branch(maybe the community can help)? And Merry Xmas! |
|
By now, I just downloaded rep. Checkout to that branch and merged it with v11.0 and uploaded it to my company's rep(so after all tests was excellent). That's an easy solution, but I hope that will be merged soon. |
|
@Me1kaa I can't give you a forecast when we will continue working on that, sorry. so yes: if it is urgent for you I would recommend to build this brand yourself for now. |
|
This is now ready to go |
note: this is a second PR directly from this repo in order to deploy it while in development
See #1210