I submitted and PR a while ago that started making warnings errors on certain
modules. Through lazy consensus it was approved and merged. I would love to
apply it to more. To set a baseline, I tried to fix most of the deprecated and
other warnings as possible in the effected code. Many were so bad off that to
just keep from getting worse I marked some suppressions. I tried to isolate to
single statements, often by extracting the offending line out into its own
method and annotating it.
Prior to this almost ever bit of free space in the warning/error gutter in
IntelliJ was take up by these warnings. The hope was by making them errors,
fixing most and suppressing where necessary that it wouldn’t get any worse. It
was hoped it would be signal to the next person in there deprecating something,
using generics, or whatever, to do it right and clean up the effected code too.
Just suppressing errors because you want to get it done is not a good practice.
Effort should be taken and only as a last resort should something be suppressed
That suppression should be limited to the smallest possible set of statements,
which may mean extracting a bit fo code into a method.
We most definitely should make it so we don’t add anymore suppressions but
absolutely not at the expense of allowing warnings. The code should be fixed or
refactored to remove the suppression.
-Jake
> On May 8, 2020, at 11:45 AM, Kirk Lund <kl...@apache.org> wrote:
>
> I'm reviewing lots of PRs which are
> adding @SuppressWarnings("deprecation"), etc. I think this is a bad trend.
> We should not be suppressing warnings in our code. That's hiding a problem
> that we'll need to or want to deal with in the future. Also, if you add
> several of these suppress annotations into a class or a method or a test,
> it really diminishes the readability. It adds noise and suppresses valid
> warnings.
>
> On one of Jinmei's PRs, she responded saying she has to add these to her
> code because of some change that Jake made to the build. Can we make it so
> we don't have to add these suppressions?
>
> Thanks,
> Kirk
