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

Marcus Hirt marcus.hirt at datadoghq.com
Sun Oct 20 12:42:35 UTC 2019


Might be better to just rename the page.html to page.template, and add
placeholders for the place(s) you want to inject stuff.

/M

On Sun, Oct 20, 2019 at 12:14 PM Marcus Hirt <marcus.hirt at datadoghq.com>
wrote:

> 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