jfx icon indicating copy to clipboard operation
jfx copied to clipboard

Remove final from TransformationList (Filtered and Sorted)

Open tobiasdiez opened this issue 5 years ago • 8 comments
trafficstars

Right now it is hard to extend the FilteredList and the SortedList as these classes are marked final. This makes it practically impossible to add convenient helper methods, or change the behavior of these classes. I agree that normally you don't need to extend them, but I also don't see any reason why you shouldn't be allowed to do so.


Progress

  • [x] Change must not contain extraneous whitespace
  • [ ] Commit message must refer to an issue
  • [ ] Change must be properly reviewed

Download

$ git fetch https://git.openjdk.java.net/jfx pull/278/head:pull/278 $ git checkout pull/278

tobiasdiez avatar Aug 02 '20 09:08 tobiasdiez

:wave: Welcome back tobiasdiez! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request.

bridgekeeper[bot] avatar Aug 02 '20 09:08 bridgekeeper[bot]

https://github.com/openjdk/jfx/pull/22#issuecomment-547337681

kleopatra avatar Aug 03 '20 10:08 kleopatra

internal review ID : 9066289

tobiasdiez avatar Aug 03 '20 10:08 tobiasdiez

thanks :)

kleopatra avatar Aug 03 '20 10:08 kleopatra

In addition to needing a JBS issue, since this is an enhancement request, with an associated API change, a PR is not the right starting point. It will need prior discussion on the openjfx-dev mailing list. See the Before submitting a pull request section of the CONTRIBUTING guidelines, specifically the part that says:

If this is a feature request, please note the additional requirements and expectations in the New features / API additions section of the Code Review Policies doc.

Many of the JavaFX classes are final by design, and in any case, it isn't as simple as "just remove the final keyword". You will need to provide motivation as to why this is needed. Our general position would be to not accept such a proposed change, but go ahead and start the discussion.

kevinrushforth avatar Aug 04 '20 01:08 kevinrushforth

Thanks for the feedback. I wasn't sure if this really falls as a "feature" or "API" change. I just send a message to the dev mailing list.

tobiasdiez avatar Aug 04 '20 12:08 tobiasdiez

@kevinrushforth I think my email is stuck in the moderation queue. Can you please have a look?

tobiasdiez avatar Aug 14 '20 06:08 tobiasdiez

@kevinrushforth Is there any update on this?

My mail to the dev list only got one response (https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-August/027395.html), which was rather general. I agree that porting some of the functionality of EasyBind back to JavaFX would be desirable in the long term, but that's a rather huge project.

tobiasdiez avatar Jan 25 '22 15:01 tobiasdiez

@tobiasdiez This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Mar 31 '23 23:03 bridgekeeper[bot]

@kevinrushforth @kleopatra Any feedback on how to proceed/what is missing is highly appreciated.

tobiasdiez avatar Apr 01 '23 05:04 tobiasdiez

There seems to be insufficient motivation to do this.

kevinrushforth avatar Apr 01 '23 13:04 kevinrushforth

@tobiasdiez This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Apr 29 '23 16:04 bridgekeeper[bot]

It's a shame that simple changes like this one here, that would simplify implementations in downstream libraries, are not appreciated.

tobiasdiez avatar Apr 30 '23 04:04 tobiasdiez

I would also like to see this PR merged.

Perhaps it should be mandated that if a class such as this is marked as final, then it be accompanied with a reason why. Otherwise, you get "justifications" like "well, I'm sure it was marked final for a reason...", 15 years later. Nothing bugs me more than a "well, that's the way it's always been..." reason for anything.

credmond avatar Nov 03 '23 00:11 credmond