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

[Jessye Coleman Shapiro]

Here is an updated patch. It includes:

- A message that reads "No 'Flag Changed' events found".  I didn't add
an explanation for why no events were found because it is possible
that the table could be empty for two reasons simultaneously- some
XFlagChanged events could be disabled and others could have simply not
occurred. However, I am not sure if this message is informative
enough.  I have placed the message under the title of the table - let
me know if this is a good location for it.
- Two uitests that test if the table is correctly populated for a
flight recording that has multiple XFlagChanged events and if the "No
'Flag Changed' events found" message appears for a flight recording
where no XFlagChanged events occur.
- Removed empty space/lines

Let me know what you think,

Jessye

On Mon, Jun 24, 2019 at 10:58 AM Ken Dobson <kdobson at redhat.com> wrote:
>
> 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
> >> > >>
> >> > >
> >> >
> >>
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jmc-5499.patch
Type: text/x-patch
Size: 23095 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/jmc-dev/attachments/20190702/68e10c2c/jmc-5499-0001.patch>