search

amlinger / homebridge-telldus-tdtool

A Homebridge plugin for Tellstick without Live, interfaced with the CLI tool tdtool.
21 stars 11 forks source link

Closes #35: Adding ESLint. #37

Closed amlinger closed 6 years ago

amlinger commented 7 years ago

This will help contributors of this repository adhere to a more similar style of code.

jannylund commented 7 years ago

I suggest using line length 120 instead of 80. I know 80 has long been considered a standard, but today it makes a lot more sense to allow 120 as screens are growing wider and wider. (and it's still a rather useful limit)

jannylund commented 7 years ago

Btw, you could consider adding a SonarQube analysis as well. Their hosted solution is free for opensource projects these days. (not sure how well it handles JS though).

https://sonarqube.com/

amlinger commented 7 years ago

Regarding 120 chars per line instead of 80, definitely agree that is an old setting (that I might be clinging to for a bit too long). I might suggest a compromise of 100 for two reasons:

  1. No need for horizontal scrolling on GitHub.
  2. Readable PR:s on mobile.

I'm not at all familiar with SonarQube, thanks for the suggestion, I'll look it up.

jannylund commented 7 years ago

100 sounds fine. SonarQube is static analyses + linting + code coverage visualization, it takes a while to get it in your workflow but personally I like it very much. It has however shown a lot of false positives in the javascript projects I use it on, but I still think the value it brings is worth, even with manual marking of false positives.

amlinger commented 6 years ago

Close to the 1-year anniversary of this I think it is nice to get some closure on this. I've changed the line limit to 100.

SonarQube looks pretty interesting, but it is out of scope for this for now. If you are still interested in setting it up, I'd be interested in following that up. Thanks!