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

handle undefined string #20

Merged
merged 1 commit into from
Mar 26, 2018
Merged

handle undefined string #20

merged 1 commit into from
Mar 26, 2018

Conversation

davisjam
Copy link
Contributor

Problem:
As reported in #19, PR #18 changed behavior on:

  • undefined string
  • non-strings

Solution:
Revert to pre-#18 behavior on non-strings: return false.

Test:
I added test cases to clarify this behavior.
It shouldn't happen again.

@davisjam
Copy link
Contributor Author

@zeke

@zeke
Copy link
Contributor

zeke commented Mar 24, 2018

Thanks! Can you update the npm test script to simply run mocha for now so we can see Travis turn green? Eventually I or someone else can remove the Makefile and the outdated component stuff to make the publish process more straightforward.

@davisjam
Copy link
Contributor Author

Will do.

@davisjam
Copy link
Contributor Author

davisjam commented Mar 24, 2018

@zeke See #22. If you merge that I will rebase this. Or we can do them in the same PR, let me know your preference.

@davisjam davisjam closed this Mar 24, 2018
@davisjam davisjam reopened this Mar 24, 2018
@zeke
Copy link
Contributor

zeke commented Mar 26, 2018

#23 has landed. If you rebase on master this should now pass.

Problem:
As reported in segmentio#19, PR segmentio#18 changed behavior on:
- undefined string
- non-strings

Solution:
Revert to pre-segmentio#18 behavior on non-strings: return false.

Test:
I added test cases to clarify this behavior.
It shouldn't happen again.
@davisjam
Copy link
Contributor Author

@zeke Rebased.

@zeke zeke merged commit a524d7f into segmentio:master Mar 26, 2018
@zeke
Copy link
Contributor

zeke commented Mar 26, 2018

$ np patch
 ✔ Prerequisite check
 ✔ Git
 ✔ Cleanup
 ✔ Installing dependencies using npm
 ✔ Running tests
 ✔ Bumping version using npm
 ✔ Publishing package
 ✔ Pushing tags

 is-url 1.2.4 published 🎉

Thanks for your perseverance. 👍

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.

2 participants