search

GoogleCloudPlatform / PerfKitExplorer

PerfKit Explorer is a dashboarding and performance analysis tool built with Google technologies and easily extensible. PerfKit Explorer is licensed under the Apache 2 license terms. Please make sure to read, understand and agree to the terms of the LICENSE and CONTRIBUTING files before proceeding.
Apache License 2.0
267 stars 64 forks source link

Transpile to ES5_STRICT, use un-minified code. #197

Closed klausw closed 9 years ago

klausw commented 9 years ago

This is initially intended for discussion - the change currently uses un-minified Angular unconditionally, not sure if that's acceptable for performance.

I've also tried to use the minified version with a source map, this now works for source code locations but not for the renamed variables, making debugging nearly impossible. So I wouldn't recommend that variant.

jmuharsky commented 9 years ago

I'm okay with non-performant code for debugging/testing purposes, but for production release we'll want minified and optimized JS.

klausw commented 9 years ago

PTAL, I've changed it so that it uses the minified code by default. Overrideable via:

appcfg update -E PKE_DEBUG:1 deploy

Even in the minified version, it now includes the source files mentioned in the source map, so source is visible.

klausw commented 9 years ago

FYI, the local compilation continues to use SIMPLE_OPTIMIZATIONS, I've added PRETTY_PRINT which shouldn't affect performance beyond increased file size. Not sure how to push the conditional into the gulpfile.

jmuharsky commented 9 years ago

Is the conditional in this case just to decide which third_party.html file(s) to include? I don't think it's necessarily the best choice to build this into the Gulp task -- it would be ideal to switch this on the fly from a deployed site.

klausw commented 9 years ago

No, I had meant to make the gulpfile conditional for the compilation settings. But this currently only affects the pretty_print setting which should be ok to leave on unconditionally - we can revisit this once we get uncompiled client code working. So I think we don't need a gulpfile conditional at this point.

On Thu, Oct 1, 2015 at 8:48 AM, jmuharsky notifications@github.com wrote:

Is the conditional in this case just to decide which third_party.html file(s) to include? I don't think it's necessarily the best choice to build this into the Gulp task -- it would be ideal to switch this on the fly from a deployed site.

— Reply to this email directly or view it on GitHub https://github.com/GoogleCloudPlatform/PerfKitExplorer/pull/197#issuecomment-144767831 .

klausw commented 9 years ago

PTAL, The tests now also run strict mode, this discovered a few more pre-existing issues.

klausw commented 9 years ago

Rebased to pick up the widgetEditorController test fix, all tests are passing now.

jmuharsky commented 9 years ago

Per discussion, please add documentation on how to set Debug mode on a test, or during deployment.

jmuharsky commented 9 years ago

Otherwise LGTM.