Review Request for JMC-4469: Adding a page from the properties view

Joshua Matsuoka jmatsuok at redhat.com
Mon Apr 29 20:26:50 UTC 2019


Hi Ken,

The patch looks good to me. If there are no objections I'll push this for
you.

Cheers,

- Josh

On Wed, Apr 3, 2019 at 10:33 AM Ken Dobson <kdobson at redhat.com> wrote:

> Oops good catch not sure how I missed that. I think I've caught them all
> now.
>
> http://cr.openjdk.java.net/~kdobson/addpagefromproperties2/webrev/
>
> Ken Dobson
>
> On Tue, Apr 2, 2019 at 11:02 PM Marcus Hirt <marcus.hirt at datadoghq.com>
> wrote:
>
> > Hi Ken,
> >
> > Thanks for the updated review!
> >
> > There is a System.out.println that you probably didn't intend to leave in
> > there:
> >
> >
> http://cr.openjdk.java.net/~kdobson/addpagefromproperties1/webrev/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrPropertySheet.java.cdiff.html
> >
> > Also, I still see funky whitespace in line endings JfrPropertySheet @@
> > -330,7 +342,19 @@.
> >
> > Kind regards,
> > Marcus
> >
> > On Tue, Apr 2, 2019 at 4:02 PM Ken Dobson <kdobson at redhat.com> wrote:
> >
> >> Thanks for the review, here's the webrev with trailing spaces removed.
> >>
> >> http://cr.openjdk.java.net/~kdobson/addpagefromproperties1/webrev/
> >>
> >> Thanks,
> >>
> >> Ken Dobson
> >>
> >> On Mon, Apr 1, 2019 at 9:44 PM Marcus Hirt <marcus.hirt at datadoghq.com>
> >> wrote:
> >>
> >>> Hi Ken!
> >>>
> >>> There are some trailing spaces that should be removed, but other than
> >>> that it looks good!
> >>>
> >>> Kind regards,
> >>> Marcus
> >>>
> >>> On Mon, Apr 1, 2019 at 7:14 PM Ken Dobson <kdobson at redhat.com> wrote:
> >>>
> >>>> Hi all,
> >>>> please review this patch for JMC-4469 to allow for adding a page with
> >>>> the
> >>>> events selected in the property view.
> >>>>
> >>>> bug: https://bugs.openjdk.java.net/projects/JMC/issues/JMC-4469
> >>>> webrev:
> >>>> http://cr.openjdk.java.net/~kdobson/addpagefromproperties/webrev/
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Ken Dobson
> >>>>
> >>>
>


More information about the jmc-dev mailing list