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

Exclude node_modules completely if deploying to google cloud functions, as GCF installs package.json. #264

Merged
merged 4 commits into from
Oct 31, 2017

Conversation

debanjanbasu
Copy link
Contributor

@debanjanbasu debanjanbasu commented Oct 30, 2017

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:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@HyperBrain
Copy link
Member

HyperBrain commented Oct 30, 2017

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?

Copy link
Member

@HyperBrain HyperBrain left a 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 :)

});
// Copy modules only if not google-cloud-functions
// GCF Auto installs the package json
if (_.get(this.serverless, 'service.provider.name') !== 'google') {
Copy link
Member

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();
}
...

Copy link
Member

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.

@HyperBrain HyperBrain merged commit dbceb1b into serverless-heaven:master Oct 31, 2017
@HyperBrain
Copy link
Member

HyperBrain commented Nov 1, 2017

@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?
I do not use Google, so I'm not able to check whether it works or not 😄
The main task for that is #128

jamesmbourne pushed a commit to jamesmbourne/serverless-webpack that referenced this pull request Oct 15, 2019
Exclude node_modules completely if deploying to google cloud functions, as GCF installs package.json.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants