-
Notifications
You must be signed in to change notification settings - Fork 3k
fix grpc method conversion in BlockingServerInterceptor #48360
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
Conversation
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
Can you replace the star imports and makes sure your code is formatted (or the CI will complain).
...ntime/src/main/java/io/quarkus/grpc/runtime/supports/blocking/BlockingServerInterceptor.java
Outdated
Show resolved
Hide resolved
...pc/runtime/src/test/java/io/quarkus/grpc/runtime/supports/BlockingServerInterceptorTest.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Can you squash your commits? |
5521a34
to
849f346
Compare
commits are squashed :) |
import io.quarkus.grpc.runtime.Interceptors; | ||
import io.vertx.core.Vertx; | ||
|
||
import static javax.lang.model.SourceVersion.isKeyword; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am pretty sure we don't want this in code that runs at runtime and not build time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im afraid there is no other way at the moment without knowing the real method name. If you mean SourceVersion specifically, i agree. That could be part of this class.
Anyway, i don't know how much use the reserved-keywords support for this interceptor is, since the extension generates mutiny stubs that are uncompilable (try with something like 'Void'). Looks like this problem could be solved in a more standardized way. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that for now we could just have a copy of the reserved keywords and be done with it
849f346
to
a7cca9a
Compare
This comment has been minimized.
This comment has been minimized.
…reserved keywords in BlockingServerInterceptor
a7cca9a
to
637bfb1
Compare
again, fixed code formatting for ci |
This comment has been minimized.
This comment has been minimized.
Status for workflow
|
Uh oh!
There was an error while loading. Please reload this page.