On Thu, 8 Jul 2021 11:19:57 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
> > the first two are naturally within the original scope, the third is near > > enough (a property on one of the covered controls) to be included .. the > > last is arguable, IMO - would tend to not include it here but open a > > follow-up to then include _all_ sync issues in the skin (probably needs > > more digging and definitely more testing). > > Thoughts? > > I reviewed the files and found those two places in files being edited in this > PR were missing the newly added null check. It is logical to add null checks > in all places in files being edited in this PR. good point, except that it's not _all_ in the skin *grinning (there are two in listeners to listView selectionModel properties) > Fixing review comments in the same PR or creating a follow-up issue is up to > the author and reviewer's agreement. > Let us not undo the change which is already done in this PR :) > I like the _not undoing_ aspect :) - so the question is: should we find and fix all in combo's skin in this issue or leave this as-is and create a follow-up for the not yet included? > If we are sure that the similar fix is needed at other places (apart from the > files which are being fixed in this PR), we can do it in a single OR multiple > follow-up issue(s). probably was unclear: didn't mean other controls - suspect there are, which we wait to bubble up sometime :) ------------- PR: https://git.openjdk.java.net/jfx/pull/557
