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

[Marcus Hirt]

Good to go! :)

/M

-----Ursprungligt meddelande-----
Från: jmc-dev <jmc-dev-bounces at openjdk.java.net> För Joshua Matsuoka
Skickat: den 29 april 2019 22:27
Till: Ken Dobson <kdobson at redhat.com>
Kopia: jmc-dev at openjdk.java.net
Ämne: Re: Review Request for JMC-4469: Adding a page from the properties view

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/appl
> ication/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jm
> c/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
> >>>>
> >>>
>