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

Feat/concurrency #680

Closed
wants to merge 32 commits into from
Closed

Conversation

vicary
Copy link
Member

@vicary vicary commented Jan 6, 2021

What did you implement:

Supersedes #517, adding concurrency support for more than one thread.

How did you implement it:

Please see #517 (comment), basically I did two changes:

  1. Replacing serizliedCompile: Boolean with concurrency: Number
  2. Mapping serializedCompile: true to concurrency: 1 for backward compatibility

How can we verify it:

Use the config below when packaging:

package:
  individually: true

custom:
  webpack:
    concurrency: 5

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

jamesmbourne and others added 30 commits January 6, 2021 19:14
* Add copyExistingArtifacts to packageModules

* Refactor packageModules

* Set service path

* Generate artifact name from service

* Output artifacts to .webpack before copying

* Set artifact name for service packaging

* Skip webpack:compile if --no-build is set

* Add webpack:package:copyExistingArtifacts hook

* Make packageModules & packExternalModules no-op if skipCompile is set

* Refactor packageModules

* Remove artifact location setting from packageModules

* Update cleanup to check this.keepOutputDirectory

* Fix path join on Windows

Co-authored-by: Miguel A. Calles MBA <[email protected]>
Use `.map(..., { concurrency })` instead of `.mapSeries(...)`
… serverless.yml.

Left only number conversion, since CLI options are typed as string
@vicary
Copy link
Member Author

vicary commented Jan 13, 2021

Shall I rebase and PR to release/5.4.0 instead of master?

ping @HyperBrain @miguel-a-calles-mba

@miguel-a-calles-mba miguel-a-calles-mba added this to the 5.4.0 milestone Jan 13, 2021
@miguel-a-calles-mba
Copy link
Member

@vicary, please rebase against release/5.4.0.
CC: @j0k3r

@vicary
Copy link
Member Author

vicary commented Jan 14, 2021

@miguel-a-calles-mba Did exactly that in #681, closing this one in favor of that. Thanks for the reply!

@vicary vicary closed this Jan 14, 2021
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.

9 participants