Skip to content

Conversation

@dfa1
Copy link
Contributor

@dfa1 dfa1 commented Aug 31, 2020

Hello,

thank you for this nice library. I would like to propose few minor fixes:

  • missing Thread.currentThread().interrupt(); after catching InterruptedException
  • converting Object.nonNull to Assertions.nonNull
  • removing some vestigial @SuppressWarnings("unchecked")
}

public <K> Builder keyContexts(List<K> keys, List<Object> keyContexts) {
nonNull(keys);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here I believe the intent was to throw NPE, correct?

- missing Thread.currentThread().interrupt(); after catching InterruptedException - converting Object.nonNull to Assertions.nonNull - removing some vestigial @SuppressWarnings("unchecked")
}

public static Throwable cause(CompletableFuture completableFuture) {
public static Throwable cause(CompletableFuture<?> completableFuture) {
Copy link
Contributor Author

@dfa1 dfa1 Aug 31, 2020

Choose a reason for hiding this comment

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

not sure about this raw type

Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

Thanks for this

@bbakerman bbakerman merged commit bca7289 into graphql-java:master Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants