[PATCH] JMC-5499 Add XFlagChanged events to JVM Information or subpage

[Alex Macdonald]

On Thu, Jun 20, 2019 at 9:05 AM Jessye Coleman Shapiro <jescolem at redhat.com>
wrote:

> Hi Alex,
>
> I will make those changes and send an updated patch.
>

Err, whoops. I was reviewing your JMC-5706 patch at the same time as this
one, and must have applied the other patch when I thought it was this one.
The previous review actually applies to your JMC-5706 patch, so I'll copy
the review on that thread.

As for this patch:

My initial reaction was that the new table uses up a good portion of the
page real-estate (which is okay), but I'm wondering if the table should
only be displayed if it has values to display. Perhaps someone else could
share their opinions as well.

There are empty spaces/lines at the following locations:
- defaultPages.xml @ line 597
- JVMInformationPage.java @ lines 122, 162, 187, 227, 231, 258, 262, 295,
296, 303, 305, 342, 346

The build & uitests pass [0], however because a new component was added to
the UI I'd like to see a uitest to ensure that everything is working as
intended.

[0] https://travis-ci.org/aptmac/jmc-qa/builds/548262559


>
> Thank you!
>
> Jessye
>
> On Wed, Jun 19, 2019 at 3:45 PM Alex Macdonald <almacdon at redhat.com>
> wrote:
>
>> Hi Jessye,
>>
>> On Fri, Jun 7, 2019 at 2:48 PM Jessye Coleman Shapiro <
>> jescolem at redhat.com> wrote:
>>
>>> Hi,
>>>
>>> This patch addresses JMC-5499: Add XFlagChanged events to JVM Information
>>> or subpage [0].  I have added a JVM Flags Log table to the JVM Internals
>>> Page that displays XFlagChanged events.
>>>
>>> Please see the attached patch and let me know what you think.
>>>
>>
>> The content of this patch look good to me.
>>
>> However, there are test failures:
>>
>> > TestRulesWithJfr.verifyAllResults:132->verifyRuleResults:159
>>
>> because the generated JfrRuleBaseline.xml doesn't match the hardcopy one
>> used to verify the test. This will need to be updated.
>>
>> There's also some empty spaces that can be cleaned up.
>>
>> JavaBlockingRule.java
>> - empty space @ lines 134, 138, 151
>> - trailing empty space @ line 142
>>
>>
>>>
>>> Thank you!
>>>
>>> Jessye Coleman-Shapiro
>>>
>>>  [0] https://bugs.openjdk.java.net/browse/JMC-5499
>>
>>
>> Cheers,
>>
>> Alex
>>
>