RFR: JMC-6555 Convert JOverflow plugin to SWT

Jie Kang jkang at redhat.com
Tue Sep 17 17:01:35 UTC 2019


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.

--- /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>"?

Apart from these, the patch looks pretty good!


Regards,


>
> Thanks to Alex for creating this webrev.
>
> Regards,
>


More information about the jmc-dev mailing list