Enforce more eslint rules #681
Conversation
xPaw
added
Type: Bug
|
Builds for this PR should pass after #684 is merged. I'll rebase this PR accordingly. |
astorije
force-pushed
the
xpaw/more-eslint
branch
from
dae63f7
to
aa02fd5
Compare
| indent: [2, tab] | ||
| key-spacing: [2, {beforeColon: false, afterColon: true}] | ||
| keyword-spacing: [2, {before: true, after: true}] | ||
| linebreak-style: [2, unix] | ||
| no-console: 0 | ||
| no-control-regex: 0 | ||
| no-inner-declarations: 2 | ||
| no-invalid-regexp: 2 | ||
| no-irregular-whitespace: 2 |
There was a problem hiding this comment.
Why removing these 3? It looks to me like we do want to keep checking on them, no?
There was a problem hiding this comment.
Theyre defaults in the recommended rules.
There was a problem hiding this comment.
Argh, I forgot to check that...
| no-trailing-spaces: 2 | ||
| no-unexpected-multiline: 2 |
There was a problem hiding this comment.
We might also want to keep this. From http://eslint.org/docs/rules/no-unexpected-multiline:
Note that the patterns considered problems are not flagged by the semi rule.
| no-trailing-spaces: 2 | ||
| no-unexpected-multiline: 2 | ||
| no-unreachable: 2 |
There was a problem hiding this comment.
Was that removed on purpose? Why?
| if (i === "userStyles") { | ||
| if (!/[\?&]nocss/.test(window.location.search)) { | ||
| $(document.head).find("#user-specified-css").html(options[i]); | ||
| (function SettingsScope() { |
There was a problem hiding this comment.
Settings with an upper case on purpose? Also, why wrapping these 70 lines?
There was a problem hiding this comment.
I wrapped them to reduce the scope (especially the I variable which was being shadowed in other functions)
| console.error(err); | ||
| fs.ensureDir(destDir, function(dirErr) { | ||
| if (dirErr) { | ||
| console.error(dirErr); |
| fonts.forEach(function (font) { | ||
| fs.copy(srcDir + font, destDir + font, function (err) { | ||
| fonts.forEach(function(font) { | ||
| fs.copy(srcDir + font, destDir + font, function(err) { |
There was a problem hiding this comment.
Urgh, I know this is to be consistent with the rest of our code, but this is painful to look at 😅. Sorry I missed this.
Also, I believe I should have used => functions there instead...
| @@ -278,6 +278,10 @@ Client.prototype.updateToken = function(callback) { | |||
| var client = this; | |||
|
|
|||
| crypto.randomBytes(48, function(err, buf) { | |||
| if (err) { | |||
| throw err; | |||
| } | |||
There was a problem hiding this comment.
Any unexpected consequences with this?
Enforce more eslint rules
More consistent style, and avoid potential bugs.