Integrated: 5904: Merge org.openjdk.jmc.ui.celleditors with org.openjdk.jmc.rjmx.ui.celleditors
Alex Macdonald
aptmac at openjdk.java.net
Thu Nov 25 19:03:07 UTC 2021
On Tue, 29 Jun 2021 16:52:20 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:
> This PR addresses JMC-5904 [[0]](https://bugs.openjdk.java.net/browse/JMC-5904), in which it is noted that org.openjdk.jmc.ui.celleditors could be merged with org.openjdk.jmc.rjmx.ui.celleditors instead of separating the code.
>
> This is achieved by combining the celleditors code into a new package; org.openjdk.jmc.ui.celleditors. Originally the intent was to throw the jmc.ui.celleditors code into rjmx.ui.celleditors. however this gets complicated because jmc.ui.misc.FilterEditor (which requires CommonCellEditors) gets used in flightrecorder.ui. As a result, moving the celleditors code into rjmx.ui.celleditors would add a dependency on rjmx.ui from flightrecorder.ui, which is not ideal.
>
> To alleviate this, FilterEditor has been moved to flightrecorder.ui (which is the only place it's used currently anyways), and celleditors have been moved to it's own package so it can be imported by rjmx.ui and jmc.ui (and subsequently flightrecorder.ui) as required. This removes the dependency from rjmx.ui to flightrecorder.ui.
>
> EDIT (November 19th): below is the original concept for this PR; to merge jmc.ui.celleditors into rjmx.ui.celleditors. However, this creates a situation where rjmx.ui becomes a dependency of flightrecorder.ui, which is not ideal. For reference, I'll leave the original PR body below.
>
> For the most part this is a fairly straightforward migration of code. There are three classes in jmc.ui.celleditors that needed to be moved into rjmx.ui.celleditors:
> 1. ClearableTextCellEditor
> 2. CommonCellEditors
> 3. NullableTextCellEditor
>
> Once these files are moved there is a slight problem: org.openjdk.jmc.ui.misc.FilterEditor uses CommonCellEditors, and org.openjdk.jmc.rjmx.ui requires org.openjdk.jmc.ui. In this case, adding a dependency from org.openjdk.jmc.rjmx.ui.celleditors.CommonCellEditors to org.openjdk.jmc.ui.misc.FilterEditors would introduce a circular dependency.
>
> The solution I've gone with here is to move FilterEditor to org.openjdk.jmc.flightrecorder.ui, which IMO makes sense anyways because it's closer to the code it gets consumed by. FilterEditor is used in:
> - _org.openjdk.jmc.flightrecorder.ui_.common.DataPageToolkit
> - _org.openjdk.jmc.flightrecorder.ui_.common.FilterComponent
> - _org.openjdk.jmc.flightrecorder.ui_.common.LaneEditor
> - _org.openjdk.jmc.flightrecorder.ui_.pages.ItemHandlerPage
>
> **Note:** this issue was originally targeted for 8.1.0, but isn't really critical (it's a P4 task), so if desired we can hold off on integrating it now and keep this around for integration into 8.2.0 as discussed in the jmc bi-weekly calls
>
> [0] https://bugs.openjdk.java.net/browse/JMC-5904
This pull request has now been integrated.
Changeset: 24f4cb0e
Author: Alex Macdonald <aptmac at openjdk.org>
URL: https://git.openjdk.java.net/jmc/commit/24f4cb0e5cbf862c51aa6eca40a24ae4312406a4
Stats: 776 lines in 67 files changed: 293 ins; 394 del; 89 mod
5904: Merge org.openjdk.jmc.ui.celleditors with org.openjdk.jmc.rjmx.ui.celleditors
Reviewed-by: hirt
-------------
PR: https://git.openjdk.java.net/jmc/pull/271
More information about the jmc-dev
mailing list