Add -Werror for javac #107
Conversation
| */ | ||
| // TODO: Remove SuppressWarnings below later. | ||
| @SuppressWarnings("JdkObsolete") | ||
| @SuppressWarnings({"JdkObsolete", "unchecked"}) |
There was a problem hiding this comment.
Why not just fix the error instead of suppressing?
There was a problem hiding this comment.
I didn't know how I can resolve the warning for creating array of LinkedLists. Since I was confident that there will be no problem with the code I just silenced it.
However, I just changed it to use List of Lists 6517fba, I don't think this will impact the performance. But if you think this will actually impact the performance, please let me know and I will undo this. Thank you.
| // just to prevent javac warnings, we currently pass "-Werror" as compiler arguments, | ||
| // all warnings must be resolved. | ||
| compileOnly deps.build.errorProneCoreOld |
There was a problem hiding this comment.
it seems once error prone is in the path we get the error below for compiling annotator-core:
warning: unknown enum constant SeverityLevel.SUGGESTION
reason: class file for com.google.errorprone.BugPattern$SeverityLevel not found
Added a dependency to resolve this issue.
| } | ||
|
|
||
| @Override | ||
| @SuppressWarnings("unchecked") |
There was a problem hiding this comment.
@msridhar The only way to resolve this warning is to change the json dependency to org.json from com.googlecode.json-simple:json-simple since it is compiled with a very old javac (the latest release is for 2012). I am definitely interested in updating this dependency to use org.json but that will include a lot of changes which makes this PR pretty large. Should I do it in a follow up PR with an issue for now to track it ?
There was a problem hiding this comment.
Just created a followup PR that removes the added @SuppressWarnings in this PR and all existing ones by changing dependency to org.json. #123.
|
@msridhar Thank you for the review, this is ready for another round. |
|
@nimakarimipour do you still need a review on this one? |
Low Priority
This PR adds
-Werrorjavac flag to builds and gets rid of all existing warnings.@msridhar @lazaroclapp this is low priority but just wanted to get rid of warnings 🙂.