kiwiirc / irc-framework

🛠️ A better IRC framework for node.js. For bots and full clients.
MIT License
181 stars 64 forks source link

Switch to eslint #226

Closed xPaw closed 4 years ago

xPaw commented 4 years ago

Mostly standard with some rules to match what's currently used in the code. This makes line endings and ident consistent, so I recommend viewing the diff with whitespace changes hidden.

Two issues I didn't fix:

  1. ping_timer in startPeriodicPing is assigned but never used, is it supposed to be cancelled somewhere?
  2. This if check doesn't use triple equals and has jshint ignore, any reason why?

https://github.com/kiwiirc/irc-framework/blob/43e313dec062a2b5f7a19f93a4738001313337e0/src/transports/net.js#L234

I backtracked it to https://github.com/prawnsalad/KiwiIRC/commit/27bca914b2b921d6b0b08e2d6afac0cc9c9cfe80 but I'm not sure what the regression is?

xPaw commented 4 years ago

I wanted to leave let/const for a future PR so it was easier to keep track of the changes.

Can you look at the two questions?

prawnsalad commented 4 years ago

ping_timer in startPeriodicPing is assigned but never used, is it supposed to be cancelled somewhere?

This was changed in https://github.com/kiwiirc/irc-framework/commit/e5c3598dcb16b3839aba82b09b311aecae6b9408 and it's no longer needed to be stored anywhere. So ping_timer can safely be removed.

This if check doesn't use triple equals and has jshint ignore, any reason why?

I remember getting confused with this line when it was originally put in way back then. Can't remember why now... @M2Ys4U if you see this, can you remember? Also, hi

xPaw commented 4 years ago

I figured it out, iconv.encode returns a Buffer, not a string.

xPaw commented 4 years ago

I just tested all encodings in iconv-lite and there isn't a single one that fails the TEST ascii check. Can we just remove it?

Gotta keep iconv.encode in a try/catch though, so it still validates that the encoding actually exists.

EDIT: Nevermind, there's things like base64, utf16 that fail the check. I will fix it and try to add tests.