npm / fstream

Advanced FS Streaming for Node
ISC License
208 stars 43 forks source link

make fstream standard #38

Closed othiym23 closed 9 years ago

othiym23 commented 9 years ago

The only oddity here is that since standard is set to check for both Node and browser compliance, some variables at the module scope level (closed, FileReader) are marked as read-only. See discussion on https://github.com/eslint/eslint/issues/892 for details. I had to get a little creative to work around this.

If this doesn't look too objectionable, we can make all the npm dependents under npm/ on GitHub follow this style (eventually).

r=@isaacs r=@feross

feross commented 9 years ago

LGTM! :smile:, but I think checking in .eslintrc to multiple repos may become a maintenance burden. It might be easier for contributors to just install the standard linter into their editor if they want that (there's already a package for Sublime Text and it's easy to support others - I'm even happy to write more if you tell me which editors to support).

And just running npm test works great too :)

isaacs commented 9 years ago

I lack the patience to go through this whole diff.

If the tests seem to work, and it doesn't break npm's tests, then LGTM.

othiym23 commented 9 years ago

\o/ One module down, eleventy hojillion to go!