Skip to content
This repository was archived by the owner on Jan 28, 2025. It is now read-only.

Add a binary to allow use without copying example #688

Merged
merged 5 commits into from
Oct 22, 2020

Conversation

tomyan
Copy link
Contributor

@tomyan tomyan commented Oct 16, 2020

This makes running builds simpler for non-serverless projects that use this library.

This makes running builds simpler for non-serverless projects that use this library.
@codecov
Copy link

codecov bot commented Oct 16, 2020

Codecov Report

Merging #688 into master will decrease coverage by 0.46%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #688      +/-   ##
==========================================
- Coverage   80.18%   79.72%   -0.47%     
==========================================
  Files          54       55       +1     
  Lines        1731     1741      +10     
  Branches      366      368       +2     
==========================================
  Hits         1388     1388              
- Misses        285      295      +10     
  Partials       58       58              
Impacted Files Coverage Δ
packages/libs/lambda-at-edge/src/command.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef2d596...165fbe0. Read the comment docs.

@dphang
Copy link
Collaborator

dphang commented Oct 16, 2020


async function main (args: string[]) {
if (args.length > 1) {
console.error("Usage: build-lambda-at-edge [ NEXT_APP_DIR ]")
Copy link
Collaborator

@dphang dphang Oct 17, 2020

Choose a reason for hiding this comment

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

Please run prettier to reformat this whole file, it seems it is not formatted as it has inconsistent semicolons at end of line (it should be part of git hook). Also if possible a simple unit test would be good too. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @dphang

Thanks for the feedback. I've pushed a commit with the requested formatting using prettier. I'm not sure how to meaningfully unit test this functionality - perhaps mocking out and injecting the Builder, but it seems this would just be a test of the implementation rather than the behaviour. Let me know if you have a suggestion and I'll look at implementing it.

Thanks

Tom

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I guess just ensure this script reasonably works as expected, mocking should be fine but if you don't think it's too useful, I'm fine as well since this is not part of the core lambda-at-edge source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started to look at this but it wasn't immediately obvious how to run the unit tests for the lambda-at-edge module. If you're happy to go with it as is then that would be fine by me, or I'm happy to write a test if you can give me some guidance on how to get the tests running. Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can merge it for now, you can check the CONTRIBUTING.md for how to run tests, we use yarn test lambda-at-edge which will run the tests there.

Thanks.

@dphang dphang merged commit 0c138a2 into serverless-nextjs:master Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants