Closed bfinney-thoughtworks closed 6 years ago
This merge request is to resolve issue#177 .
Hi @bfinney-thoughtworks, welcome first of all, and thanks!
There is another way of doing this by adding the exception at the top of the file instead of doing it for each statement. What do you think?
This file has an example: https://github.com/rabblerouser/core/blob/master/backend/bin/seed.js
Hi @bfinney-thoughtworks, welcome first of all, and thanks!
Thank you for the welcome :-)
There is another way of doing this by adding the exception at the top of the file instead of doing it for each statement.
That has a different effect: it disables the check for every statement in the file. It would reduce the scope of checking for actual mistakes in the code. I think that's not desirable.
For that reason, directives to disable static checks should be as narrow as possible, to only open the hole where it is needed, and leave the rest of the code subject to checks.
I generally agree with narrowing the scope when disabling static checks. In this case I'd would sacrifice that (for test files only) over having a line of comment per assertion given that we use chai a lot.
I think I agree with @pameck here. I'd love to see something like this merged so that we can get rid of those annoying warnings, but I think I prefer the one-comment-at-the-top-of-the-file solution, especially seeing as it will only apply to test files.
Thanks @bfinney-thoughtworks and sorry for the long delay!
Just realised those were the last warnings left since we switched to a stricter eslint config, so now we're warning-free! Amazing!
The idiom in a test using Chai is to use
expect(…).to.be.true
and similar; see <URL:http://www.chaijs.com/api/bdd/>. This looks like an unused expression; disable that check for these test files.