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 <[email protected]> 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 <[email protected]> on behalf of Johan Vos > <[email protected]> > *Date: *Monday, December 4, 2023 at 09:25 > *To: *Kevin Rushforth <[email protected]> > *Cc: *[email protected] <[email protected]> > *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 <[email protected]> > 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 > > > > > >
