refactor: remove tildes #3266
refactor: remove tildes #3266
Conversation
beyackle
requested review from
a-b-r-o-w-n,
boydc2014,
corinagum,
cwhitten,
srinaath and
tonyanziano
as code owners
| //add the lu file that are not in interuption folder. | ||
| files.forEach((file) => { | ||
| if (!~paths.indexOf(file.name)) { | ||
| if (!paths.includes(file.name)) { |
There was a problem hiding this comment.
This becomes a O(ab) with the lookup inside for loop. Can paths be a Set?
There was a problem hiding this comment.
Figured most of this code was already there. If u have time for a little cleanup will be great
| ~types.indexOf(ExpressionTypeMapString[returnType]) || | ||
| (returnType === ReturnType.Number && ~types.indexOf(ExpressionType.integer)) | ||
| types.includes(ExpressionTypeMapString[returnType]) || | ||
| (returnType === ReturnType.Number && types.includes(ExpressionType.integer)) |
There was a problem hiding this comment.
cant this just be types[ExpressionType.integer] instead of doing includes everytime?
There was a problem hiding this comment.
Looking at this again, I'm not sure we can do anything about it yet. Types is an array but ExpressionType.integer is a string; we want to check if the string is a value in the array, so includes is appropriate here. The more correct thing here would be to make types a Set<string>, but that ought to be its own cleanup ticket later - it's a non-trivial amount of work.
|
@beyackle consider adding the no-bitwise rule to our eslint config as well. |
That's an excellent idea. I didn't know that existed, but I'll add it in. |
beyackle
requested review from
benbrown,
carlosscastro and
luhan2017
as code owners
|
@srinaath can you revisit this PR today? |
|
PR is good on my end. Once @a-b-r-o-w-n approves i think we are good to merge |
* remove tildes * add no-bitwise rule * update luPublisher with typo fixes and some cleanup * fix typo * remove tildes * add no-bitwise rule * update luPublisher with typo fixes and some cleanup * fix typo * fix endpoint issue
4 participants
Description
This replaces tildes across the codebase with easier-to-read equivalents (e.g.
~array.indexOf(x)becomesarray.includes(x)).Task Item
refs #3307