Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

Use path.join to form windows-compatible path #122

Merged
merged 2 commits into from
Aug 22, 2017
Merged

Conversation

FokkeZB
Copy link
Contributor

@FokkeZB FokkeZB commented Aug 18, 2017

Not sure if this causes:
https://secure.helpscout.net/conversation/415601400/628033/?folderId=20363#thread-1123745601

But still good practice not to use os-specific path separator.

@FokkeZB FokkeZB self-assigned this Aug 18, 2017
@FokkeZB
Copy link
Contributor Author

FokkeZB commented Aug 18, 2017

It also looks like this check won't work on Windows:

|| filePath.match(/aws-sdk\/apis\/.*\.json/)

Copy link
Contributor

@BrunoBernardino BrunoBernardino left a comment

Choose a reason for hiding this comment

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

Nice catch. Seems simple and straightforward enough. 👍

@FokkeZB
Copy link
Contributor Author

FokkeZB commented Aug 21, 2017

@BrunoBernardino what do you think about

|| filePath.match(/aws-sdk\/apis\/.*\.json/)
? Should I also update that to be cross-platform?

@BrunoBernardino
Copy link
Contributor

I think that makes sense!

@@ -113,7 +113,7 @@ const forceIncludeDumbPath = (filePath/*, smartPaths*/) => {
// we include smartPaths just incase you want to check the inclusion of some library
return (
(filePath.endsWith('package.json') || filePath.endsWith('definition.json'))
|| filePath.match(/aws-sdk\/apis\/.*\.json/)
|| filePath.match(path.sep === '\\' ? /aws-sdk\\apis\\.*\.json/ : /aws-sdk\/apis\/.*\.json/)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BrunoBernardino since Travis doesn't support Windows (yet) it's hard to add a test, but I've tested this logic in Parallels:

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good enough to me!

@FokkeZB FokkeZB merged commit 493c2cb into master Aug 22, 2017
@FokkeZB FokkeZB deleted the windows-compat-path branch August 22, 2017 14:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants