fix all the remaining eslint warnings #545
Conversation
commit: |
|
|
||
| "@typescript-eslint/unbound-method": "error", | ||
|
|
||
| "@typescript-eslint/no-non-null-assertion": "warn", |
There was a problem hiding this comment.
I am personally not a fan of this one 😅
if someone uses a non-null assertion I think that's generally for a good reason so having to add an eslint disabling comment for it feels very unnecessary/annoying to me 🤷
There was a problem hiding this comment.
Yeah I'm fine with this
We just need to verify on every PR to make sure it's not used unwisely
| "sonarjs/elseif-without-else": "warn", | ||
| "sonarjs/no-duplicate-string": "warn", | ||
| "sonarjs/cognitive-complexity": "warn", | ||
| "sonarjs/cognitive-complexity": ["warn", 35], |
There was a problem hiding this comment.
35 allows most functions in the codebase, I could just add disabling comment to each one instead if preferred (there were around 10-ish or something I think)
I did not attempt on splitting the functions as I am not really familiar with the codebase here (but I am happy to give it a go if you want 🙂)
There was a problem hiding this comment.
I agree.
I need to take a closer look at the PR later today, I remember that there was a reason that it was this order. Might be old code
| }; | ||
| } | ||
|
|
||
| const indexHandler = "index.hander"; |
There was a problem hiding this comment.
sorry 😓
Typos happens to everyone. Your PR made the code more DRY and easier to read. Great work ⭐️
3 participants
continuation from #544 🙂
Some changes might be controversial I am happy to adjust them any way you prefer, I just applied what I thought more sensible and/or my personal preference 🙂