Skip to content
This repository was archived by the owner on Nov 13, 2021. It is now read-only.

feat(lib): Cross-platform compatability #6

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

BenBeattieHood
Copy link
Contributor

I needed to run your superb npm package on Windows, so would like to propose these small changes to allow this please.

@BenBeattieHood
Copy link
Contributor Author

(cc @vvo)

@vvo
Copy link
Owner

vvo commented Dec 7, 2020

@BenBeattieHood Thanks! Do you have experience in testing CDK constructs? Only asking because a good way to ensure your work and mine will not break in the future would be to setup an example CDK construct and build it on windows/linux on GitHub actions.

@@ -123,15 +123,17 @@ export class NodejsFunction extends lambda.Function {
};
}, {});

const nodeifyPath = (path: string) => path.replace(/\\/g, '\\\\');
Copy link
Owner

Choose a reason for hiding this comment

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

Can you explain why this is needed + add a comment maybe? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍 good call out.

@BenBeattieHood
Copy link
Contributor Author

@BenBeattieHood Thanks! Do you have experience in testing CDK constructs? Only asking because a good way to ensure your work and mine will not break in the future would be to setup an example CDK construct and build it on windows/linux on GitHub actions.

Only a little, and only a little on GitHub actions too. This might change next year, but sorry not to be of much use right now.

@vvo vvo merged commit fded3cf into vvo:master Dec 15, 2020
@vvo
Copy link
Owner

vvo commented Dec 15, 2020

🎉 This PR is included in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@vvo vvo added the released label Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants