[
https://issues.apache.org/jira/browse/METRON-615?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15730011#comment-15730011
]
Justin Leet commented on METRON-615:
------------------------------------
Yeah, I agree about new (and even current) contributors. Whatever we do,
documenting it for contributors is important, because I also don't want people
to get turned off because things blow up that they don't expect.
There should also be documentation around what tools we're using and how we
enable / disable things like this. These particular things aren't restrictive
while something is being built or improved, but I can see cases where you'd
want to disable/downgrade errors or warnings while you're working locally. To
the best of my knowledge, this doesn't exist in a fleshed out form and should
probably end up on the wiki somewhere.
> 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)
