-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
chore: fix eslint no-undef #1462
Conversation
addresses #1461
@@ -5,6 +5,11 @@ module.exports = { | |||
"env": { | |||
"node": true | |||
}, | |||
"globals": { | |||
"Map": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't these be false by default?
looks like So should we switch to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good (contingent on travis). Two suggestions to narrow the no-console
scope
@@ -16,6 +16,8 @@ | |||
*/ | |||
'use strict'; | |||
|
|||
/* eslint-disable no-console */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since there's only one console.log
in here for a specific reason, but it's mostly a log.*()
file, does it make sense to limit to // eslint-disable-next-line no-console
over line 285?
@@ -17,6 +17,8 @@ | |||
|
|||
'use strict'; | |||
|
|||
/* eslint-disable no-console */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just // eslint-disable-next-line no-console
for just the one line in this file too?
yep. will do on both
…On Thu, Jan 12, 2017 at 2:59 PM, Brendan Kenny ***@***.***> wrote:
***@***.**** approved this pull request.
looking good (contingent on travis). Two suggestions to narrow the
no-console scope
------------------------------
In lighthouse-extension/app/src/lighthouse-background.js
<#1462 (review)>
:
> @@ -16,6 +16,8 @@
*/
'use strict';
+/* eslint-disable no-console */
since there's only one console.log in here for a specific reason, but
it's mostly a log.*() file, does it make sense to limit to //
eslint-disable-next-line no-console over line 285?
------------------------------
In lighthouse-core/index.js
<#1462 (review)>
:
> @@ -17,6 +17,8 @@
'use strict';
+/* eslint-disable no-console */
maybe just // eslint-disable-next-line no-console for just the one line
in this file too?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1462 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACMdEo_LNZt9u1dRHRl7oZeoBCZFBp3Rks5rRrBogaJpZM4LiOSs>
.
|
🎉🎆🎉 |
* update to bring in `eslint:recommended` as base config
addresses #1461