[Enhancement] JMC-6589 Bundle the d3 library for flame charts with JMC

Marcus Hirt marcus.hirt at datadoghq.com
Sun Oct 20 10:14:20 UTC 2019


Hi Miro,

Thank you for looking into this. A few nits, after a very superficial look:
* Why keep page.html.orig around? Is it still being used somewhere?
* The comments you have for where you fetched the javascript bundles differ
in that only the last one has a space between the beginning of the comment
and the text. All of them should be in the same format (personally I like
the space).

And then a general comment:
I guess inlining the Javascript into the HTML is one of the few options we
have, but wouldn't you want to have a more "unique" string to go look for?
Many of the keys you use are short and look like they, if we're unlucky,
could perhaps be used in one of the libraries we include in the future. I'd
suggest namespacing them with something like jmc.inline just to reduce the
likelihood of any future collisions.

Kind regards,
Marcus



On Fri, Oct 18, 2019 at 10:52 PM Miro Wengner <miro.wengner at gmail.com>
wrote:

> Hi Everyone
> please review my enhancement for flamechart
>
> webrev:http://cr.openjdk.java.net/~mwengner/JMC-6589/webrev/ <
> http://cr.openjdk.java.net/~mwengner/JMC-6589/webrev/>
> bug:https://bugs.openjdk.java.net/browse/JMC-6589 <
> https://bugs.openjdk.java.net/browse/JMC-6589>
>
> Kind Regards,
> Miro


More information about the jmc-dev mailing list