[External] : Re: eclipse warnings

[Andy Goryachev]

Thank you, Johan.

Just to clarify, the "eclipse" is used by the umbrella task https://bugs.openjdk.org/browse/JDK-8297300, the actual changes come under their own specific titles.  Very low priority.

-andy



From: Johan Vos <johan.vos at gluonhq.com>
Date: Thursday, December 7, 2023 at 02:37
To: Andy Goryachev <andy.goryachev at oracle.com>
Cc: Kevin Rushforth <kevin.rushforth at oracle.com>, openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
Subject: [External] : Re: eclipse warnings
That's a good question. I guess it's a compromise.
From a backport-point-of-view, 1 PR (hence 1 JBS issue) per file is ideal. But that is not realistic of course, as it would clutter the JBS issues and the github PR's.
I think package-based PR's would be a good compromise. That would also make it feasible to review in depth.

Note that I think it would be better to change the title from "Eclipse warnings" into "Coding enhancements". I don't think we want Eclipse to decide the coding approaches in OpenJFX, as we can have follow-up PR's then about "NetBeans warnings" or "IntelliJ warnings" :)

- Johan

On Wed, Dec 6, 2023 at 5:06 PM Andy Goryachev <andy.goryachev at oracle.com<mailto:andy.goryachev at oracle.com>> wrote:
Dear Johan:

What would be your best recommendation to minimize the burden?  Split into small PRs on a per-module or per-package basis?

Thanks,
-andy



From: openjfx-dev <openjfx-dev-retn at openjdk.org<mailto:openjfx-dev-retn at openjdk.org>> on behalf of Johan Vos <johan.vos at gluonhq.com<mailto:johan.vos at gluonhq.com>>
Date: Monday, December 4, 2023 at 09:25
To: Kevin Rushforth <kevin.rushforth at oracle.com<mailto:kevin.rushforth at oracle.com>>
Cc: openjfx-dev at openjdk.org<mailto:openjfx-dev at openjdk.org> <openjfx-dev at openjdk.org<mailto:openjfx-dev at openjdk.org>>
Subject: Re: eclipse warnings
Also, these commits often affect many files at once (in scattered locations), and that makes backports harder.

- Johan

On Mon, Dec 4, 2023 at 6:14 PM Kevin Rushforth <kevin.rushforth at oracle.com<mailto:kevin.rushforth at oracle.com>> wrote:
We did a few of these sort of cleanup fixes a year or so ago.

In general, this sort of cleanup *might* be useful, but also causes some code churn and takes review cycles to ensure that there is no unintentional side effect.

The last two might be OK cleanup tasks, but I wouldn't make them a high priority. Worth noting is that a seemingly redundant null check or instanceof check is not always a bad thing, so I wouldn't clean up all of them.

The first group is the more interesting one. In some cases a potential null access can highlight actual bugs. However, I oppose any automated solution for these, since adding a null check where you don't expect a null (even if you IDE thinks it might be possible) can hide the root cause of a problem.

We aren't going to enforce these, though, so you'll likely need to configure your IDE to be less picky.

-- Kevin
On 12/4/2023 8:34 AM, Andy Goryachev wrote:
Dear colleagues:

Imported the openjfx project into another workspace with a more stringent error checking and discovered a few issues:


  *   potential null pointer access: 295
  *   unnecessary cast or instanceof: 190
  *   redundant null check: 61

Do we want to clean these up?

-andy


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231207/84137b75/attachment-0001.htm>