RFR: 7308: Move non-Eclipse dependant classes from org.openjdk.jmc.ui.common to org.openjdk.jmc.common [v6]

[Brice Dutheil]

On Tue, 16 May 2023 14:15:08 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

>> This PR addresses JMC-7308 [[0]](https://bugs.openjdk.java.net/browse/JMC-7308), in which it would be helpful to have some of the classes currently in jmc.ui.common shipped in core.
>> 
>> There are a number of classes currently in jmc.ui.common that would be a great asset to the core distribution (and the third-party applications that consume jmc-core), and these classes could live in jmc.common.
>> 
>> It isn't as straightforward as moving all of the packages to core, as there are still classes in these jmc.ui.common packages that have dependencies on Eclipse or rjmx. Having said that, the ones listed below can be moved without much difficulty:
>> 
>> - org.openjdk.jmc.ui.common.action (3)
>>     Executable, IActionProvider, IUserAction
>> 
>> - org.openjdk.jmc.ui.common.jvm (5)
>>     Connectable, JVMArch, JVMCommandLineToolkit, JVMDescriptor, JVMType
>> 
>> - org.openjdk.jmc.ui.common.resource (2)
>>     IImageResource, Resource
>> 
>> - org.openjdk.jmc.ui.common.security (10)
>>     ActionNotGrantedException, CredentialsNotAvailableException, FailedToSaveException, ICredentials, InMemoryCredentials, ISecurityManager, PersistentCredentials, SecurlyStoredByteArray, SecurityException, SecurityManagerFactory
>> 
>> - org.opendjk.jmc.ui.common.tree (3)
>>     IArray, IChild, IParent
>> 
>> - org.openjdk.jmc.ui.common.util (4)
>>     Environment, Filename, ICopyable, IObservable
>> 
>> - org.openjdk.jmc.ui.common.xydata (5)
>>     DataSeries, DefautlTimestampedData, DefaultXYData, ITimeStampedData, IXYData
>> 
>> [0] https://bugs.openjdk.java.net/browse/JMC-7308
>
> Alex Macdonald has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits:
> 
>  - add last missing copyright year update
>  - add missing license year updates
>  - Merge branch 'master' of https://git.openjdk.org/jmc into PR-7308
>  - fix RjmxTestCase imports
>  - update license headers to 2023
>  - Merge master
>  - fix imports for PersistentCredentials
>    
>    I must have missed this when I rebased onto the master branch. Fixing it
>    up here.
>  - update license headers to 2022
>  - update license headers
>  - update agent plugin imports
>  - ... and 8 more: https://git.openjdk.org/jmc/compare/ae2fbf35...2536c4a7

tl;dr Apart from classes in `.action`, `.resource`, `.tree`, `.util.ICopyable`, `.util.IObservable`. This looks like nice change in particular for classes like `JVMArch`.

I'm not convinced the mentioned classes of `org.openjdk.jmc.ui.common` should go in `org.openjdk.jmc:common`. 
* Some of them are clearly related to GUI, e.g. `IUserAction`, or `ICopyable` which lands in the even more general `util` package. * `Resource`, while not strictly related to GUI seems really tied to Eclipse plugin.
* Even the interfaces in the `tree` package are somewhat odd there.

I believe the mentioned classes should 
* move either in a different module, or 
* in a `ui` subpackage (eg. `org.openjdk.jmc.common.ui.action`), or
* even remain where they currently are.

I don't think these GUI related classes have significant value to _clutter_ the core/common module.

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/util/IObservable.java line 36:

> 34: 
> 35: import java.util.Observable;
> 36: import java.util.Observer;

**thought:** Should this `IObservable` be even removed, both `Observer` and `Observable` are deprecated since JDK 9.

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

PR Review: https://git.openjdk.org/jmc/pull/300#pullrequestreview-1430238379
PR Review Comment: https://git.openjdk.org/jmc/pull/300#discussion_r1196223117