RFR: JMC-6555 Convert JOverflow plugin to SWT

[Arvin Kangcheng Xu]

No. I don't have access to a Mac or Windows machine at the moment.

On Wed, 2 Oct 2019 at 16:42, Marcus Hirt <marcus.hirt at datadoghq.com> wrote:
>
> Have you tried running this on Mac?
>
> /M
>
>
> On Wed, Oct 2, 2019 at 9:42 PM Marcus Hirt <marcus.hirt at datadoghq.com> wrote:
> >
> > Excellent! Will take a look!
> >
> > /M
> >
> > On Wed, Oct 2, 2019 at 4:07 PM Arvin Kangcheng Xu <kxu at redhat.com> wrote:
> >>
> >> Hello all,
> >>
> >> This is the latest updated patch. This is the latest patch that includes a fix
> >> for the lazy table viewer. I rebased and made sure it applies to the current
> >> HEAD, 198:ceedb367dc18.
> >>
> >> Webrev:
> >> https://cr.openjdk.java.net/~jkang/jmc-6555/webrev.05/
> >>
> >> Thanks to Jie for creating this webrev.
> >>
> >> Regards,
> >>
> >>
> >> On Wed, 18 Sep 2019 at 09:53, Arvin Kangcheng Xu <kxu at redhat.com> wrote:
> >> >
> >> > On Tue, 17 Sep 2019 at 13:02, Jie Kang <jkang at redhat.com> wrote:
> >> > >
> >> > > On Tue, Sep 10, 2019 at 4:02 PM Arvin Kangcheng Xu <kxu at redhat.com> wrote:
> >> > > >
> >> > > > This is the latest updated patch for fixes of the following, as
> >> > > > mentioned earlier in this thread:
> >> > > >
> >> > > >  - wildcard imports were replaced with single class imports
> >> > > >  - unnecessary white spaces were removed
> >> > > >  - indentations were changed to using tabs instead of spaces
> >> > > >  - removed mIsUpdatingModel guard
> >> > > >  - removed getHeapSize and mHeapSize in BaseViewer
> >> > > >  - declared setHeapSize in BaseViewer abstract
> >> > > >  - initialized mHeapSize to 1 to avoid division by zero
> >> > > >  - numbers are now rounded instead of truncated
> >> > > >  - number displays are now comma-separated
> >> > > >  - removed global jfx dependencies (javafx.osgi, p2 repo, target platforms)
> >> > > >  - refactored sub-component calls
> >> > > >  - used a more contrasting color palette for pie charts
> >> > > >  - fixed rotating table color
> >> > > >  - memory column displays 2 decimal places. add tooltips
> >> > > >  - fixed JavaThingPage NPE
> >> > > >
> >> > > > Spotbug config typos were resolved in another issue and are now fixed and push.
> >> > > >
> >> > > > Webrev:
> >> > > > http://cr.openjdk.java.net/~aptmac/JMC-6555/webrev.03/
> >> > >
> >> > > Hey Arvin,
> >> > >
> >> > > Thanks for your continued efforts here! A few more small things below from me:
> >> > >
> >> > > --- old/application/org.openjdk.jmc.joverflow.ui/src/main/java/org/openjdk/jmc/joverflow/ui/HeapDumpAction.java
> >> > > 2019-09-10 15:31:43.946263444 -0400
> >> > > +++ new/application/org.openjdk.jmc.joverflow.ui/src/main/java/org/openjdk/jmc/joverflow/ui/HeapDumpAction.java
> >> > > 2019-09-10 15:31:43.864262593 -0400
> >> > > -import java.io.File;
> >> > > -
> >> > > -import javax.management.MBeanServerConnection;
> >> > > -import javax.management.ObjectName;
> >> > > -
> >> > > import org.eclipse.jface.dialogs.InputDialog;
> >> > > import org.eclipse.jface.window.Window;
> >> > > import org.eclipse.swt.SWT;
> >> > > import org.eclipse.swt.widgets.Display;
> >> > > import org.eclipse.swt.widgets.FileDialog;
> >> > > -
> >> > > import org.openjdk.jmc.common.io.IOToolkit;
> >> > > import org.openjdk.jmc.rjmx.IConnectionHandle;
> >> > > import org.openjdk.jmc.rjmx.IServerHandle;
> >> > > @@ -56,10 +50,14 @@
> >> > > import org.openjdk.jmc.ui.misc.DialogToolkit;
> >> > > import org.openjdk.jmc.ui.misc.DisplayToolkit;
> >> > > +import javax.management.MBeanServerConnection;
> >> > > +import javax.management.ObjectName;
> >> > > +import java.io.File;
> >> > >
> >> > > The import order change here should not be made. This applies
> >> > > elsewhere in the patch. I'm guessing an auto-formatter was used but
> >> > > it's configuration for imports wasn't set correctly. If it's the
> >> > > configuration included in the jmc repository that's a minor issue that
> >> > > could be addressed separately.
> >> >
> >> > I mainly use another IDE for development and imported included Eclipse
> >> > formatting configuration there. It seems the imported config from Eclipse is
> >> > not well-supported. I've reorganized class imports for all files with Eclipse.
> >> >
> >> > > --- /dev/null 2019-09-10 09:22:37.353999759 -0400
> >> > > +++ new/application/org.openjdk.jmc.joverflow.ui/src/main/java/org/openjdk/jmc/joverflow/ui/JavaThingPage.java
> >> > >  2019-09-10 15:31:50.541331853 -0400
> >> > > @@ -0,0 +1,134 @@
> >> > >
> >> > > ++public class JavaThingPage extends Page implements ModelListener {
> >> > > + private final JOverflowEditor mEditor;
> >> > > + private JavaThingTreeViewer mTreeViewer;
> >> > >
> >> > > JavaThingTreeViewer is defined as "public class JavaThingTreeViewer<T
> >> > > extends JavaThingItem> extends TreeViewer {". Can mTreeViewer instance
> >> > > by typed to "JavaThingTreeViewer<JavaThingItem>"?
> >> >
> >> > Yes. mTreeViewer instance is now typed to JavaThingTreeViewer<JavaThingItem>.
> >> >
> >> > Since these changes are a bit trivial. I'd refrain from spamming the
> >> > mail list with
> >> > another patch. These changes will be included in my next update. Thank you
> >> > very much!
> >> >
> >> > > Apart from these, the patch looks pretty good!
> >> > >
> >> > >
> >> > > Regards,
> >> > >
> >> > >
> >> > > >
> >> > > > Thanks to Alex for creating this webrev.
> >> > > >
> >> > > > Regards,
> >> > > >