RFR: 5904: Merge org.openjdk.jmc.ui.celleditors with org.openjdk.jmc.rjmx.ui.celleditors [v4]

Alex Macdonald aptmac at openjdk.java.net
Fri Aug 20 13:08:28 UTC 2021


On Thu, 19 Aug 2021 21:13:25 GMT, Jie Kang <jkang at openjdk.org> wrote:

>> Sure I'll give this a try.
>> 
>> So what I'm having trouble with here is that `FilterEditor` ultimately needs to be used by `flightrecorder.ui.common`
>> 
>> FilterEditor requires celleditors, which in HEAD is split between `org.openjdk.jmc.ui.celleditors` and `org.openjdk.jmc.rjmx.ui`. The only celleditors class that FilterEditor needs is CommonCellEditors which is in jmc.ui, so there's no dependency on rjmx.ui. rjmx.ui also has a dependency on jmc.ui.
>> 
>> So we have:
>> 
>> jmc.ui.celleditors.celleditors->jmc.ui.misc.FilterEditor->flightrecorder.ui.common
>> jmc.ui -> jmc.rjmx.ui
>> 
>> 
>> If we're consolidating celleditors into rjmx.ui, then we have:
>> 
>> jmc.rjmx.ui -> jmc.ui.misc.FilterEditor -> flightrecorder.ui.common
>> jmc.ui -> jmc.rjmx.ui
>> 
>> 
>> In this case, we can break the cycle by moving FilterEditor elsewhere else. In the approach here I placed it in flightrecorder.ui because that's ultimately where it gets used anyways. But either way I guess it'd go rjmx.ui.celleditors -> "something" -> flightrecorder.ui, so it's a matter of being a direct dependency or not.
>> 
>> If instead we consolidate celleditors into jmc.ui.celleditors, then we have:
>> 
>> jmc.ui -> jmc.rjmx.ui
>> jmc.rjmx.services + jmc.rjmx.subscription.internal -> jmc.ui.misc.FilterEditor -> flightrecorder.ui.common
>> 
>> because jmc.ui.celleditors would need the apporpriate files that rjmx.ui had access to from rjmx.
>
> So I do `a -> b` as in a depends on b, but I think you went the opposite is that right? I'm getting confused lol...
> 
> I think in the second case of consolidating celleditors into jmc.ui, a bunch of stuff appears that wasn't discussed before (rjmx.services, rjmx.subscription) Where are they coming from? Can they be moved to somewhere better?

Er, yeah I guess I was thinking more of a flow diagram, I have it backwards here.

The reason it  looks like most of the classes were in rjmx.ui anyways was because they use classes from rjmx. So if the classes are moved from rjmx.ui to jmc.ui, they'd still need access to those (specifically rjmx.services and subscription.internal), so we'd have to add a dependency on those to jmc.ui.

-------------

PR: https://git.openjdk.java.net/jmc/pull/271


More information about the jmc-dev mailing list