RFR: JMC-5473: Quick search for Automated Analysis Result Table

Jie Kang jkang at redhat.com
Tue Sep 24 17:26:32 UTC 2019


On Wed, Sep 18, 2019 at 10:59 AM Alex Macdonald <almacdon at redhat.com> wrote:
>
> On Tue, Sep 17, 2019 at 5:20 PM Jie Kang <jkang at redhat.com> wrote:
>>
>> On Tue, Sep 3, 2019 at 2:57 PM Alex Macdonald <almacdon at redhat.com> wrote:
>> >
>> > On Wed, Aug 28, 2019 at 11:36 AM Jie Kang <jkang at redhat.com> wrote:
>> >>
>> >> On Wed, Aug 28, 2019 at 10:46 AM Jie Kang <jkang at redhat.com> wrote:
>> >> >
>> >> > Hi all,
>> >> >
>> >> > Carmine has requested I continue this work in his stead. Please find
>> >> > an updated webrev for review below.
>> >> >
>> >> > Webrev: http://cr.openjdk.java.net/~jkang/jmc-5473/webrev.00/
>> >> > Bug: https://bugs.openjdk.java.net/browse/JMC-5473
>> >>
>> >> Ah; I had a mistake in the previous webrev (the TableViewer was no
>> >> longer set), please find an updated one below. Apologies for the
>> >> mistake.
>> >>
>> >> http://cr.openjdk.java.net/~jkang/jmc-5473/webrev.01/
>> >>
>> >>
>> >
>> > Looks good - the functionality works as it should and the uitests are passing on both my Linux & Windows machines.
>> >
>> > There is an unused import that can be cleaned up at:
>> >
>> > ResultTableUi.java
>> > - unused import GridData @ line 50
>>
>> Hi Alex,
>>
>> Nice catch thanks. I've removed the unused import.
>>
>> >
>> > And my only other nit is that the test is placed in "org.openjdk.jmc.flightrecorder.uitest.overview" which would be a new package that would only have this test inside of it. It doesn't test (yet at least) the entire Automated Analysis page so maybe it doesn't fit under "[..].flightrecorder.uitest.pages", but maybe it belongs under [..].flightrecorder.uitest alongside the other tests? Just a thought.
>>
>> Sure; I've moved it to be under o.o.jmc.flightrecorder.uitest. Here's
>> the updated webrev:
>
>
> Great, thanks. LGTM.
>
>>
>>
>> http://cr.openjdk.java.net/~jkang/jmc-5473/webrev.02/
>>
>> How does it look?

Thanks for the review Alex. Could I get a second reviewer for this?


Regards,

>>
>>
>> Regards,
>>


More information about the jmc-dev mailing list