-
Notifications
You must be signed in to change notification settings - Fork 417
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
Exclude node_modules completely if deploying to google cloud functions, as GCF installs package.json. #264
Conversation
Hi @debanjanbasu , thank you for the PR. Can you fill out the description template completely - especially the checkboxes and check "enable edits from maintainers" in your PR? |
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.
Only a small semantical change :)
lib/packExternalModules.js
Outdated
}); | ||
// Copy modules only if not google-cloud-functions | ||
// GCF Auto installs the package json | ||
if (_.get(this.serverless, 'service.provider.name') !== 'google') { |
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.
I would prefer an early exit here if the provider is google instead of putting the default case into the if.
if (_.get(this.serverless, 'service.provider.name') === 'google') {
return BbPromise.resolve();
}
...
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.
Updated this. I will add unit tests for the new behavior to restore coverage.
@debanjanbasu May I ask you for a favor? There is still the column for GCF missing in the provider compatibility chart (README). Would you mind to prepare a PR that adds the GCF status to the README? |
Exclude node_modules completely if deploying to google cloud functions, as GCF installs package.json.
Exclude node_modules completely if deploying to google cloud functions, as GCF installs package.json.
Added a check to see if the provider is Google
How can we verify it:
Todos:
Is this ready for review?: YES
Is it a breaking change?: NO