Let's try this again :P.
+1 to Kirk's comments. Plus...
Another tip (for IJ IDEA users, probably same for Eclipse and other IDEs):
You can disable inspection for a warning that is otherwise benign (or not
correct) *rather than* unnecessarily annotating the code with
@SuppressWarnings.
However, disable inspections judiciously. Warnings in code are telling you
something that needs to be paid attention to and disabling the warning with
(@SuppressWarnings) or disabling the inspection makes it easy to lose sight
that the warning is there when the code is revisited.
-j
On Fri, May 8, 2020 at 11:55 AM Jacob Barrett <jbarr...@pivotal.io> wrote:
> 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
>
>
--
-John
Spring Data Team
