Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #50 #51

Closed
wants to merge 1 commit into from
Closed

Conversation

andyperlitch
Copy link

No description provided.

The [stream](https://nodejs.org/api/stream.html#stream_stream) or [buffer](https://nodejs.org/api/buffer.html#buffer_class_buffer) of the file. For example:
```js
if (file.isBuffer()) {
var contents = file.contents.toString('utf8')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var inside an if is bad JS

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@contra no it isn't and it's actually correct as per node styleguide

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phated guess i got brainwashed by crockford

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol i was actually thinking about this while writing the example but then i was like mehhhh adding a var declaration above seemed like too much...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .toString argument is also moot as it's the default.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 34bfc80 on andyperlitch:improve-api#50 into 01ed2ef on wearefractal:master.

yocontra pushed a commit that referenced this pull request Jun 3, 2015
@stevemao
Copy link

This is fixed on the master

@yocontra yocontra closed this Jul 10, 2015
@yocontra
Copy link
Member

@stevemao guess GH doesn't close multiple issues per commit msg

@stevemao
Copy link

No, https://help.github.com/articles/closing-issues-via-commit-messages/
You have to say closes #34, closes #23

phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
phated pushed a commit that referenced this pull request Sep 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants