[
https://issues.apache.org/jira/browse/METRON-615?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15729953#comment-15729953
]
Otto Fowler commented on METRON-615:
------------------------------------
I agree with promoting those to errors. Anything we can do to help with
correctness and consistency is time well spent. Also, another point I would
make is how it helps new contributors. This proposal, and things like the
coding guide give the reason behind why the code is written the way it is, and
why it is done consistently throughout the project. That kind of understanding
give new contributors more confidence in their submittals, and make the
project easier to understand and work with.
> Prevent warnings in code from future contributions
> --------------------------------------------------
>
> Key: METRON-615
> URL: https://issues.apache.org/jira/browse/METRON-615
> Project: Metron
> Issue Type: Improvement
> Reporter: Justin Leet
> Priority: Minor
>
> After cleaning up most of the warnings in METRON-612, there's a question of
> locking out additional warnings in the code from Error Prone. This leads to
> the more general question of avoiding the addition of warnings in the future
> in general, not just from Error Prone but from javac or any other tooling we
> may add.
> The Hadoop project itself does something pretty extensive, e.g. [Hadoop
> QA|https://issues.apache.org/jira/browse/HADOOP-13827?focusedCommentId=15678418&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15678418].
> Settings up something like that might be pretty involved and overkill for
> where we are, but it might be a nice end goal.
> As an parallel measure, we can potentially turn Error Prone's warnings into
> errors for warnings we've stamped out. METRON-612's PR ([Github
> PR|https://github.com/apache/incubator-metron/pull/389]), assuming no
> additional warnings have come in on master, allows for 4 to be upgraded.
> [SynchronizeOnNonFinalField|http://errorprone.info/bugpattern/SynchronizeOnNonFinalField]
> [BoxedPrimitiveConstructor|http://errorprone.info/bugpattern/BoxedPrimitiveConstructor]
> [ClassCanBeStatic|http://errorprone.info/bugpattern/ClassCanBeStatic]
> [ClassNewInstance|http://errorprone.info/bugpattern/ClassNewInstance]
> Of these, I'm personally in favor of promoting SynchronizeOnNonFinalField,
> BoxedPrimitiveConstructor, and ClassNewInstance. ClassCanbeStatic I'm pretty
> neutral on.
> In particular, SynchronizeOnNonFinalField is a correctness issue.
> BoxedPrimitiveConstructor is a perf / code quality issue. ClassNewInstance
> avoids some issues around reflection. Both BoxedPrimitiveConstructor and
> ClassNewInstance are expected to have the originating problems become
> deprecated in Java 9 in favor of the more correct construction.
> [~dlyle]'s opinion per the PR:
> {quote}
> Totally For Erroring on:
> SynchronizeOnNonFinalField
> Not against it for:
> BoxedPrimitiveConstructor
> ClassCanBeStatic
> Okay with it for ClassNewInstance with additional input. I get why it's
> harmful, but is it more harmful than reflection in general? Or do you just
> want to not insert known future deprecated code?
> {quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
