Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nsqadmin: eslint fixes #1080

Merged
merged 2 commits into from
Sep 5, 2018
Merged

nsqadmin: eslint fixes #1080

merged 2 commits into from
Sep 5, 2018

Conversation

mreiferson
Copy link
Member

@mreiferson mreiferson commented Sep 4, 2018

Feels like a good time to land this after #856

Some of the fixes in the Handlebars helpers for graphite look like they'll fix real bugs :)

if (key === 'gc_runs') {
target = 'movingAverage(' + target + ',45)';
}
var fullKey = formatStatsdKey(metricType(key), target);
targets.push(target);
Copy link
Member Author

@mreiferson mreiferson Sep 4, 2018

Choose a reason for hiding this comment

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

e.g. this one (using target not fullKey)

@@ -88,11 +91,10 @@ var genTargets = function(typ, node, ns1, ns2, key) {
if (node === '*') {
t = 'averageSeries(' + t + ')';
}
var fullKey = formatStatsdKey(metricType(key), t);
return 'scale(' + t + ',0.000001)';
Copy link
Member Author

Choose a reason for hiding this comment

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

and this one (using t not fullKey)

Copy link
Member

@jehiah jehiah left a comment

Choose a reason for hiding this comment

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

changes look reasonable

Copy link
Member

@ploxiln ploxiln left a comment

Choose a reason for hiding this comment

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

This all looks good.

Checking this out locally and using eslint v5.2.0 I still get these things, which just need to be ignored somehow:

$ eslint static/js/

/Users/pierce/go/src/github.com/nsqio/nsq/nsqadmin/static/js/app_state.js
   7:24  warning  'VERSION' is not defined                no-undef
   8:29  warning  'GRAPHITE_URL' is not defined           no-undef
   9:30  warning  'GRAPH_ENABLED' is not defined          no-undef
  10:32  warning  'STATSD_INTERVAL' is not defined        no-undef
  11:38  warning  'STATSD_COUNTER_FORMAT' is not defined  no-undef
  12:36  warning  'STATSD_GAUGE_FORMAT' is not defined    no-undef
  13:30  warning  'STATSD_PREFIX' is not defined          no-undef
  14:27  warning  'NSQLOOKUPD' is not defined             no-undef
  16:25  warning  'IS_ADMIN' is not defined               no-undef
  17:26  warning  'BASE_PATH' is not defined              no-undef

/Users/pierce/go/src/github.com/nsqio/nsq/nsqadmin/static/js/lib/ajax_setup.js
  7:24  warning  'USER_AGENT' is not defined  no-undef

✖ 11 problems (0 errors, 11 warnings)

@mreiferson mreiferson merged commit 8f6fa1f into nsqio:master Sep 5, 2018
@mreiferson mreiferson deleted the js-lint branch September 5, 2018 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants