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

[Ken Dobson]

I agree, a simple string added to the table saying "No X events found" or
"X event has been disabled" would more than suffice.

Ken


On Mon, Jun 24, 2019 at 7:07 AM Marcus Hirt <marcus.hirt at datadoghq.com>
wrote:

> Agree. Perhaps now would be a good time to discuss?
>
> I am, in general, hesitant to automatically hide UI elements. It has been
> tried in the past (e.g. Microsoft hiding menu elements not used much), with
> little success. That said, we should at the very least have some feedback
> showing people whether the lack of data is due to events being disabled or
> events not having occurred. What do you guys think?
>
> Kind regards,
> Marcus
>
> On Thu, Jun 20, 2019 at 8:35 PM Ken Dobson <kdobson at redhat.com> wrote:
>
>> There's a ticket here about how we should handle empty tables
>> https://bugs.openjdk.java.net/browse/JMC-4009
>> I think it's an issue that might have a larger scope than just this bug
>> but
>> it's definitely worth addressing.
>>
>> Ken Dobson
>>
>> On Thu, Jun 20, 2019 at 1:30 PM Alex Macdonald <almacdon at redhat.com>
>> wrote:
>>
>> > 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
>> > >>
>> > >
>> >
>>
>