KAFKA-3375; Suppress deprecated warnings where reasonable and tweak compiler settings#1042
KAFKA-3375; Suppress deprecated warnings where reasonable and tweak compiler settings#1042ijuma wants to merge 7 commits into
Conversation
|
Review by @granthenke and @gwenshap maybe? |
There was a problem hiding this comment.
You mean "remove unchecked"?
There was a problem hiding this comment.
I meant adding unchecked like so:
options.compilerArgs << "-Xlint:deprecation,unchecked"
|
LGTM. Maybe @ewencp could check if some unchecked casting in connect can be avoided? |
|
@guozhangwang I don't think they can, but happy for @ewencp to take a look. |
There was a problem hiding this comment.
Do we want to globally enable implicit conversions? Since we don't use them a lot (I think) and we are favoring JavaConverters vs JavaConversions we could just use import scala.language.implicitConversions where necessary.
There was a problem hiding this comment.
I think so, there isn't anything wrong with implicit conversions as extension methods IMO (implicit value classes give you that with no performance penalty). I can remove it if people feel strongly, but the language imports seem like useless boilerplate in most cases.
There was a problem hiding this comment.
I am okay either way. Just wanted to mention it.
There was a problem hiding this comment.
I'm also ok with the import, but it is complete 180 turn on what we did before (only import the converters in the smallest possible scope to avoid any accidental conversion or something).
Perhaps a community discussion on the list about reversing this convention? I'm not suggesting a vote, but an announcement and give people chance to object if they feel strongly about it?
There was a problem hiding this comment.
Explained to Gwen that this change doesn't affect the JavaConverters/JavaConversions thing.
|
LGTM. Thanks for the cleanup @ijuma |
Ignored Kafka Streams on this iteration.
ijuma
force-pushed
the
kafka-3375-suppress-depreccated-tweak-compiler
branch
from
5667ff9 to
90d3322
Compare
|
Added a trivial additional commit that applies @granthenke's suggestion to reuse |
|
LGTM |
4 participants
@SafeVarargsannotation to fix warningsOnce we drop Java 7 and Scala 2.10, we can tweak the compiler settings further so that they warn us about more things.