Skip to content

Migrating and isolating google analytics to tolerate failure to load#379

Merged
stefanoborini merged 6 commits into
masterfrom
369-google-analytics
Mar 30, 2017
Merged

Migrating and isolating google analytics to tolerate failure to load#379
stefanoborini merged 6 commits into
masterfrom
369-google-analytics

Conversation

@stefanoborini

@stefanoborini stefanoborini commented Mar 28, 2017

Copy link
Copy Markdown
Contributor

Fixes #369

@codecov-io

codecov-io commented Mar 29, 2017

Copy link
Copy Markdown

Codecov Report

Merging #379 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #379   +/-   ##
======================================
  Coverage    95.3%   95.3%           
======================================
  Files          92      92           
  Lines        3941    3941           
  Branches      255     255           
======================================
  Hits         3756    3756           
  Misses        128     128           
  Partials       57      57

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc74f46...bf8372a. Read the comment docs.

@stefanoborini stefanoborini changed the title Migrating and isolating google analytics to tolerate failure to load [WIP] Migrating and isolating google analytics to tolerate failure to load Mar 29, 2017
Comment thread jstests/tests/test_analytics.js Outdated
result[0] = [cmd, id, auto];
};
window.apidata.analytics = undefined;
var ga = analytics.init();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not using ga variable after that, just analytics.init(); is enough I think.

Comment thread jstests/tests/test_analytics.js Outdated
result[0] = [cmd, id, auto];
};
window.apidata.analytics = {"tracking_id": "X"};
var ga = analytics.init();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

if (window.apidata.analytics !== undefined) {
window.ga('create', window.apidata.analytics.tracking_id, 'auto');
} else {
window.ga = function () {

@martinRenou martinRenou Mar 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think window.ga = function () { return; } and window.ga = function () {}; will have the same effect but for the second option jslint would complain about empty brackets

@stefanoborini stefanoborini Mar 30, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pycharm complains for the exact opposite reason. It claims return is unnecessary.
by the way, I use jshint, not jslint (jslint is the one by Douglas Crockford IIRC, and while I respect the guy, I think his linter is a bit excessive)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahah ok :)

@stefanoborini stefanoborini merged commit 9284647 into master Mar 30, 2017
@stefanoborini stefanoborini deleted the 369-google-analytics branch March 30, 2017 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants