Skip to content

Conversation

agnusha
Copy link

@agnusha agnusha commented Jul 1, 2025

Official documentation: https://www.serverless.com/framework/docs/guides/plugins/cli-output
Connected with issue #263

@silouone silouone self-requested a review July 3, 2025 08:17
@agnusha
Copy link
Author

agnusha commented Jul 10, 2025

@silouone Hello, is it possible to get a review?

@joaolrpaulo
Copy link

joaolrpaulo commented Aug 1, 2025

Hello @silouone any ETA for this to be reviewed & merged?

Copy link
Member

@silouone silouone left a comment

Choose a reason for hiding this comment

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

Hello Thanks for taking interest and the PR, and sorry for the long wait without answers.

The technical approach looks ok and correctly implements serverless v4 compatibility and the new logging system.

I would need to try it out on our internal stack since we heavily required on serveless, it would be a good use case to test that out.

also please consider this feedback:

  • Please squash commits into 2-3 logical units.
    • "feat: update to serverless v4 plugin API"
    • "test: improve integration test reliability"
    • "chore: update dependencies and version bumps"

Or else.

  • Please consider the few feedback I made about log being potentially undefined and support for that.
  • Please add doculmentation for this migration, after #installation:

Migrating from v7 to v8

This version requires Serverless Framework v4 and serverless-offline v14.4.0+.

Breaking Changes

  • Requires serverless-offline: "^14.4.0 || >=14"
  • Uses new Serverless v4 plugin API for logging
  • Node.js 18+ required

Upgrade Steps

  1. Update serverless to v4: npm install serverless@^4.0.0
  2. Update serverless-offline to v14.4.0+: npm install serverless-offline@^14.4.0
  3. Update this plugin: npm install serverless-offline-s3@^8.0.0

@silouone silouone added the enhancement New feature or request label Aug 3, 2025
@agnusha
Copy link
Author

agnusha commented Aug 4, 2025

@silouone thanks a lot for your review. I really appreciate it. I will take a look at the provided feedback.

@agnusha agnusha force-pushed the master branch 8 times, most recently from 9253d6b to 24dd7e0 Compare August 8, 2025 12:09
@agnusha
Copy link
Author

agnusha commented Aug 8, 2025

@silouone Thank you for the thorough review. I’ve carefully addressed all the comments.

@MichalOleszczuk
Copy link

Hi,
There is error right now running auto-creation of queue i created branch with fix but i can not publish it for review.
Can you please tell me how can i present PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants