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

Jump to bottom

Create unit tests for sanitizePath #1352

Merged
merged 11 commits into from Aug 16, 2017
Merged

Create unit tests for sanitizePath #1352

merged 11 commits into from Aug 16, 2017

Conversation

retrohacker
Copy link
Member

  • Create unit tests for sanitizePath

@DonutEspresso
Copy link
Member

Hmm is there overlap between this and the plugins sanitize path plugin?

@retrohacker
Copy link
Member Author

There is, whats more is that it doesn't look like the util.sanitizePath function is used anywhere. I'm 👍 on moving this implementation over to the plugin and migrating the tests to that API. The plugin's implementation is done with a for loop.

William Blankenship added 5 commits July 28, 2017 14:05
Merge branch '5.x' into sanitizePathTests
617d54c
sanitizePath plugin unit tests
9ed372e 
* Remove unused utils.santizePath implementation
* Remove test for utils.sanitizePath
* Add test for sanitizePath plugin
Merge branch 'sanitizePathTests' of github.com:restify/node-restify i…
61fc92e 
…nto sanitizePathTests
Revert mergeQs breakout
b03099a
revert .js suffix
fc0307d
@retrohacker
Copy link
Member Author

@alex ready for another review 😄

@DonutEspresso
Copy link
Member

LGTM! Rebase and let's get this in. 🎉

@DonutEspresso DonutEspresso self-requested a review August 16, 2017 06:23
@retrohacker retrohacker merged commit 12714cf into 5.x Aug 16, 2017
@retrohacker retrohacker deleted the sanitizePathTests branch August 17, 2017 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DonutEspresso DonutEspresso DonutEspresso approved these changes

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@retrohacker @DonutEspresso