-
Notifications
You must be signed in to change notification settings - Fork 416
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
Don't package non-node functions (fix for #644) #663
Conversation
Could you rebase against the master? Thanks! |
Also, could you add some tests to cover your changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test to cover that update? Thanks 🙏
Yes, I've been looking at the unit tests to figure out where to put it and what to check (as I would have expected the existing unit tests to fail, but they aren't). I think I've got that figured out, so I will try and get that in today. |
…heaven#644 Also fixed a typo in one of the names of the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks!
What did you implement:
Most of serverless-webpack already deals with not packaging non-node functions. There is only one section that is still problematic. This fix deals with the issue where a runtime may be defined for an individual function or for a provider. It will default to a runtime of "nodejs" if no runtime is specified at either the function or provider level.
Closes #644
How did you implement it:
When processing the async config, implemented an extra step before mapping the functions to filter out functions that don't have a node runtime.
How can we verify it:
Add a function (in my case it was a Rust function) that uses a non-node runtime and the app should still package correctly.
Todos:
Is this ready for review?: YES
Is it a breaking change?: NO