-
Notifications
You must be signed in to change notification settings - Fork 295
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
Initial support for JDK 11 #263
Conversation
@@ -294,6 +294,11 @@ public void externalInitexternalInitSupportFields() { | |||
|
|||
@Test | |||
public void generatedAsUnannotated() { | |||
String generatedAnnot = |
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.
javax.annotation.Generated
has disappeared in JDK 11 🙁
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.
Is there an equivalent? What do people use instead?
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.
There is javax.annotation.processing.Generated
now, though there is no consensus on what to use (see e.g. grpc/grpc-java#3633 for discussion). Here we just use javax.annotation.processing.Generated
in our tests if we're on JDK 11
@lazaroclapp I don't fully understand the coveralls integration. Any chance you can confirm that it is still configured correctly? Now we will be running two different Travis jobs (and likely will do so for a while) so not sure about what the config should be. |
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.
Honestly, I am not sure about the coveralls status. I am not even sure it was well configured to begin with. I can take a look at that later, but, for now, this looks good to land to me :)
For now, only the
:nullaway:test
target is passing on CI. We need to address issues outlined in #259 to get everything working, and it may be a while before the sample Android app can easily be built. That said, I think this initial level of support is useful and worth landing.