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

Modern logs for Serverless Framework v3 #1013

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

medikoo
Copy link
Contributor

@medikoo medikoo commented Nov 24, 2021

Configure modern logs with provided new logging interface, maintaining compatibility with v2 release of a Framework (this PR ensures no changes to log output happens for that version).

New logs can be tested with serverless@pre-3 release

After v3 is released, we may consider dropping support for v2, and cleaning up the code so only modern logs are configured - I can help providing a PR

v2 Logs (before)

success:
Screenshot 2021-10-13 at 17 49 56

warning:
Screenshot 2021-10-13 at 17 50 25

error:
v2-error

v3 Logs (after)

progress:
Screenshot 2021-10-13 at 17 50 37

success:
Screenshot 2021-10-13 at 17 50 08

warning:
v3-warning

error:
v3-error

lib/cleanup.js Show resolved Hide resolved
lib/logStats.js Outdated Show resolved Hide resolved
lib/logStats.js Outdated Show resolved Hide resolved
lib/packExternalModules.js Show resolved Hide resolved
lib/packageModules.js Outdated Show resolved Hide resolved
lib/packageModules.js Outdated Show resolved Hide resolved
lib/packagers/index.js Show resolved Hide resolved
lib/run.js Show resolved Hide resolved
lib/wpwatch.js Outdated Show resolved Hide resolved
@j0k3r j0k3r added this to the 5.7.0 milestone Dec 1, 2021
lib/compile.js Outdated Show resolved Hide resolved
@j0k3r
Copy link
Member

j0k3r commented Dec 1, 2021

Could you also add build specific to Serverless v3? Like the one we have for Serverless v1?

@medikoo
Copy link
Contributor Author

medikoo commented Dec 1, 2021

Could you also add build specific to Serverless v3? Like the one we have for Serverless v1?

@j0k3r I'm not sure exactly to what build you're referring. Can you provide more details?

@j0k3r
Copy link
Member

j0k3r commented Dec 1, 2021

I'm not sure exactly to what build you're referring. Can you provide more details?

We have a dedicated build for Serverless v1 (other build use the v2).

serverless-v1:
runs-on: ${{ matrix.os }}
name: Node.js ${{ matrix.node }} on ${{ matrix.os }} with Serverless v1
strategy:
matrix:
os:
- ubuntu-20.04
node:
- "14"
steps:
- name: "Checkout"
uses: actions/checkout@v2
with:
fetch-depth: 2
- name: "Install Node.js"
uses: actions/setup-node@v2
with:
node-version: "${{ matrix.node }}"
- name: "Install Serverless v1"
run: npm install serverless@1
- name: "Install dependencies"
run: npm ci
- name: "Run tests"
run: "npm run test"

Adding that kind of stuff for Serverless v3, maybe it could interesting to have a build dedicated to that version too.

@medikoo
Copy link
Contributor Author

medikoo commented Dec 1, 2021

We have a dedicated build for Serverless v1 (other build use the v2).

Currently, we have just pre-release of v3, but I agree it's good to add it to testing. I've setup dedicated job

@j0k3r
Copy link
Member

j0k3r commented Dec 2, 2021

The build using Serverless pre-release is failing. Do you think it's too soon to include it?
If there are more work to be done on serverless-webpack for the v3 compatibility, maybe we can postpone that build for later, what do you think?

There is one last change to apply #1013 (comment) and we'll be good to go

@medikoo medikoo force-pushed the 1123-v3-logs branch 2 times, most recently from f277364 to 55dde61 Compare December 2, 2021 09:10
@medikoo
Copy link
Contributor Author

medikoo commented Dec 2, 2021

The build using Serverless pre-release is failing. Do you think it's too soon to include it?

Great thanks for pointing I've missed that.

I've fixed it, it was a question of providing some data which in v3 is required by Serverless constructor (and passing it to older version is safe). So it's good to have addressed

@j0k3r
Copy link
Member

j0k3r commented Dec 2, 2021

Great, just rebase against the master and it'll be good!

@medikoo
Copy link
Contributor Author

medikoo commented Dec 2, 2021

Sure, rebased, thank you!

Copy link
Member

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Thanks for all the work 🤝

@j0k3r
Copy link
Member

j0k3r commented Dec 2, 2021

Oh sh*t. We won't be able to merge because of the coverage drops 🤦‍♂️
I'll ask to remove the required on it.

@mnapoli
Copy link
Contributor

mnapoli commented Jan 6, 2022

@j0k3r FYI we are closing in on the v3 stable release. Not sure how we could help to move this forward, let us know

@mnapoli
Copy link
Contributor

mnapoli commented Jan 17, 2022

Quick update: v3 release is next week

@j0k3r
Copy link
Member

j0k3r commented Jan 17, 2022

Thanks @mnapoli.
As I said to @medikoo, we are stuck until a reply from @HyperBrain to unlock PRs. Sorry.

@j0k3r
Copy link
Member

j0k3r commented Mar 24, 2022

Hello @medikoo sorry for being late, the coveralls check is no more required.
Can you rebase and fix conflicts and we'll (finally) be able to merge your work!

@medikoo
Copy link
Contributor Author

medikoo commented Mar 24, 2022

@j0k3r updated!

@j0k3r j0k3r merged commit 0e83424 into serverless-heaven:master Mar 24, 2022
@j0k3r
Copy link
Member

j0k3r commented Mar 24, 2022

Finally 🥳

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.

3 participants