Rewrite module, remove/upgrade deps, clean up code and add test+docs+standard style

[LinusU]

Hi @stephen,

I don't know exactly how it happened but I rewrote your entire library 😆

My initial plan was to just drop express because I wanted something really lightweight to run as smoothly as possible on embedded hardware. But from there I kind of started refactoring and tried to do slim the library down further.

• [x] Removed express and ip dependencies
• [x] Upgraded to icy instead of the old icecast-stack
• [x] Cleaned up the code
• [x] Added tests
• [x] Wrote documentation
• [x] Added standard style

I'm sending this pull request since I would rather that there be one library instead of two very similar. If you don't want to accept this, I'll understand and I'll publish under my own name. Otherwise I would really appreciate it if you could would accept this pull request and add me as a maintainer here and on npm (my name is linusu on npm).

Thanks! Linus

[LinusU]

Oh, actually, this should solve some bugs from before as well:

• #10 - This is now working correctly again
• #7 - This is no longer needed since I use the incoming socket of the request to determine which address and port to give out in the playlist. This has the benefit of working on multiple interfaces at the same time, with multiple clients from multiple interfaces. Basically, it just works™ :)

[LinusU]

ping @stephen 😄

[LinusU]

Friendly reminder @stephen 💌

[stephen]

@LinusU - +1 on these changes, though I'm not a fan of the standard linting. I'd prefer to stick with the airbnb styleguide as airsonos does (though probably using eslint over jscs).

Also +1 on adding you on as a maintainer after merging, and we can get CI setup to run tests. 😄

[LinusU]

😱 semicolons;

No but sure, I'll add airbnb linting as well. Will hopefully get it done this weekend 👍 thanks for responding :)

[yoiang]

hey @LinusU , was there anything special I need to do to get this working? I switched over the two calls (start and stop to listen and close) but don't get any playback!

[LinusU]

Wow I have totally missed following up on this, sorry about that... I will try to fix it this weekend...

@yoiang Not that I remember, it worked for out of the box. Hopefully I can test again this weekend 👌

[LinusU]

So I spent some time to convert this to follow the Airbnb styleguide, but I'm not that happy with the result. First of all, eslint only works on Node.js 4 and up, but I intended to support Node.js 0.10 and 0.12 as well. This will make the CI fail for Node 0.10 and 0.12, standard has fixed this by only running on newer versions, and exiting with a success exit status and a small message on unsupported version. Here I would need to implement this myself.

The next problem is that the airbnb styleguide focuses on ES6+, which again is a problem if you want to support 0.10 and 0.12. You can mitigate this by using airbnb/legacy, but there is still a number of problems that I think arise from ES6 first thinking:

• no-underscore-dangle - Before we had Symbol support, a very common way of marking a instance variable as "private" in javascript was to prefix it with an underscore. Most people know and understand this.
• no-param-reassign - Before we had support for default parameter values, a very common pattern was to write lines like name = name || 'Default name' as the first line of a function, in order to simulate default parameter values.

There was also a few other rules which I think made this code worse, instead of better.

• no-unused-vars - While I actually like this rule, and it's enabled in standard, it's configured a bit stricter here. It complains on the following code, which is used to preserve the arity of the function.

Nicercast.prototype.listen = function (port, hostname, backlog, callback) {
  this._server.listen.apply(this._server, arguments)
}

• no-console - We are using console.log in the example (example.js). I understand why you don't want to console.log in library code, but this styleguide is supposed to line both libraries and applications...

• no-empty - While often good it prevents using the quite common pattern try { ... } catch (_) {} (an empty catch block) to ignore errors from a specific line...

• quote-props - I think that you should stay consistent within an object, even if some keys don't require quoting. Example below:

  res.writeHead(200, {
    'Connection': 'close',
    'Content-Type': 'audio/mpeg',
    'Icy-Metaint': (acceptsMetadata ? META_INTERVAL : undefined)
  })

I would therefor like to stick with standard-style. If this is not acceptable to you, I can understand that, I will publish my code under some other name and keep maintaining it there. You are of course free to use any of my code, merge this and change the style, or do anything else you please with it, it's all open source. Hope you understand my position :)

[LinusU]

Travis is failing with "ENETUNREACH"... no idea why 🤔