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

Marcus Hirt marcus.hirt at datadoghq.com
Mon Jun 24 11:07:03 UTC 2019


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
> > >>
> > >
> >
>


More information about the jmc-dev mailing list