Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update README.md to fix sample config. #265

Merged
merged 2 commits into from
Oct 31, 2017

Conversation

thepont
Copy link
Contributor

@thepont thepont commented Oct 30, 2017

Path needs to be an absolute path since webpack 2.3.1 current sample code in documentation will not run.

- configuration.output.path: The provided value ".webpack/service" is not an absolute path!

What did you implement:

Fix for a sample in the documentation, webpack requires path to be absolute.

Closes #XXXXX

How did you implement it:

Starting with the sample code, I adjusted it so it worked. using path.resolve to resolve the abs path for web pack.

How can we verify it:

Check that sample correctly runs by running the webpack.config.js

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

Path needs to be an absolute path since webpack 2.3.1, sample code will not run.

`- configuration.output.path: The provided value ".webpack/service" is not an absolute path!`
@thepont thepont changed the title Update README.md Update README.md to fix sample config. Oct 30, 2017
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.

Hi @thepont , great find 🙌 and thank you for the PR.
Could you add the filename change too?

README.md Outdated

module.exports = {
// ...
output: {
libraryTarget: 'commonjs',
path: '.webpack',
path: path.resolve(__dirname, '.webpack'),
filename: 'handler.js', // this should match the first part of function handler in `serverless.yml`
Copy link
Member

Choose a reason for hiding this comment

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

Since 3.x.x the filename should also be defined as [name].js so that it can be used with automatic entry resolution, which is now the preferred way to set the entries.

Copy link
Member

Choose a reason for hiding this comment

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

Changed it right now - ready to merge 😄

@HyperBrain HyperBrain added this to the 4.0.0 milestone Oct 31, 2017
@HyperBrain HyperBrain merged commit 0c650a0 into serverless-heaven:master Oct 31, 2017
HyperBrain pushed a commit that referenced this pull request Oct 31, 2017
Update #265

Added #260

Added release notes. Increased version.
jamesmbourne pushed a commit to jamesmbourne/serverless-webpack that referenced this pull request Oct 15, 2019
Update README.md to fix sample config.
jamesmbourne pushed a commit to jamesmbourne/serverless-webpack that referenced this pull request Oct 15, 2019
Update serverless-heaven#265

Added serverless-heaven#260

Added release notes. Increased version.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants