Skip to content

feat: Add --s3-bucket and --s3-prefix to sam delete#3915

Merged
hawflau merged 20 commits intoaws:developfrom
lucashuy:s3_delete
Jun 8, 2022
Merged

feat: Add --s3-bucket and --s3-prefix to sam delete#3915
hawflau merged 20 commits intoaws:developfrom
lucashuy:s3_delete

Conversation

@lucashuy
Copy link
Contributor

@lucashuy lucashuy commented May 27, 2022

Which issue(s) does this change fix?

#3246

Why is this change necessary?

Adding S3 bucket options will provide the ability to delete S3 buckets and objects if the bucket and prefix cannot be obtained from the local config file or CloudFormation template.

How does it address the issue?

This change adds the --s3-bucket and --s3-prefix command line options to sam delete.

What side effects does this change have?

This change adds additional parameters to sam delete related components.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Add input/output type hints to new functions/methods
  • Write design document if needed (Do I need to write a design document?)
  • Write/update unit tests
  • Write/update integration tests
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. area/delete labels May 27, 2022
@lucashuy lucashuy changed the title Add --s3-bucket and --s3-prefix to sam delete feat: Add --s3-bucket and --s3-prefix to sam delete May 27, 2022
Copy link
Contributor

@mndeveci mndeveci left a comment

Choose a reason for hiding this comment

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

LGTM in general! 🥳 Added couple of small comments.

@mingkun2020 mingkun2020 added pr/internal and removed pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels May 30, 2022
Copy link
Contributor

@mndeveci mndeveci left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@qingchm qingchm left a comment

Choose a reason for hiding this comment

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

LGTM! Good job on following up with comments and drafting the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants