search

memcachier / memjs

A memcache client for node using the binary protocol and SASL authentication
MIT License
197 stars 52 forks source link

Linting #76

Closed miccolis closed 8 years ago

miccolis commented 8 years ago

PR has eslint run as part of npm test and fixes the various style inconsistencies it finds. This should probably be extended to the test code as well, but I wanted to float the idea of adding this before making the PR larger.

alevy commented 8 years ago

Looks like there is an issue installing a dependency of eslint called chalk with node version 0.8

alevy commented 8 years ago

If that's hard to fix, I wouldn't mind merging the lint changes themselves without adding the linter to the test suite yet (although that would obviously be even better)

miccolis commented 8 years ago

@alevy yea, it's looking like trying to get eslint working with node 0.8 is a non-starter. I'm happy to remove the linting from npm test, but before I do... any chance I can convince you to deprecate v0.8 support? The last release was july 2014 and it's no longer maintained at all. That said I understand a change like that would demand major release, etc...

alevy commented 8 years ago

@miccolis I can definitely be pushed that way. Heroku (which hosts the vast majority of users of memjs I'm aware of) no longer supports 0.8 and after asking around a bit it seems like the right thing to do. I'm doing some tweaks with travis in a separate branch, so I think we can go a head and merge this, and deprecate 0.8 separately

miccolis commented 8 years ago

Sounds good, let me know if you need anything else here.