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

Remove redundant empty string handing in _.toArray() #2349

Merged
merged 1 commit into from
Nov 18, 2015

Conversation

captbaritone
Copy link
Collaborator

As part of the review for pull request #2298, @jdalton requested that we add
this check to handle the empty string case. However, I'm guessing he didn't
realize that the first line of the function, if (!obj) return [];, already
handles that case.

As part of that pull request, @JonAbrams wisely added a test for the empty
string case, so as long as tests are passing this change should be safe.

See: #2298 (comment)

As part of the review for pull request jashkenas#2298, @jdalton requested that we add
this check to handle the empty string case. However, I'm guessing he didn't
realize that the first line of the function, `if (!obj) return [];`, already
handles that case.

As part of that pull request, @JonAbrams wisely added a test for the empty
string case, so as long as tests are passing this change should be safe.

See: jashkenas#2298 (comment)
@captbaritone
Copy link
Collaborator Author

Tests are failing for an unrelated reason:

$ nvm install 0.10

Version '0.10' not found - try `nvm ls-remote` to browse available versions.

The command "nvm install 0.10" failed and exited with 3 during .

@captbaritone
Copy link
Collaborator Author

Closed and reopened the pull request to trigger a Travis rebuild. Tests are now green.

@JonAbrams
Copy link
Contributor

Nice catch!

michaelficarra added a commit that referenced this pull request Nov 18, 2015
Remove redundant empty string handing in _.toArray()
@michaelficarra michaelficarra merged commit 3699e39 into jashkenas:master Nov 18, 2015
@michaelficarra
Copy link
Collaborator

Thanks.

@captbaritone
Copy link
Collaborator Author

No problem. Thanks for the great library.

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.

3 participants