-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: remove duplicate code and fix indentation #8011
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
Conversation
samcli/lib/bootstrap/bootstrap.py
Outdated
""" | ||
message = f"\n\tManaged S3 bucket: {s3_bucket}" | ||
click.secho(message, bold=bold) | ||
click.echo("\tA different default S3 bucket can be set in samconfig.toml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an opportunity to improve on the message as well.
click.echo("\tA different default S3 bucket can be set in samconfig.toml") | |
click.echo("\tTo use a specify bucket, specify `resolve_s3 = false` and `s3_bucket= <bucket arn>`") |
Then we can remove the condition in line 57 and 58. We can consolidate these to one message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can definitely improve on the messaging and removing branching overall.
* Set the right indentation across `package`, `deploy`, `package/deploy --resolve-s3` & `deploy --guided` * Set the right indentation on `Skipping upload` to S3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing!
package
,deploy
,package/deploy --resolve-s3
&deploy --guided
Skipping upload
to S3.Which issue(s) does this change fix?
No explicit issue against this.
Why is this change necessary?
The indentation was not aligned.
BEFORE:

AFTER:

How does it address the issue?
What side effects does this change have?
Mandatory Checklist
PRs will only be reviewed after checklist is complete
make pr
passesmake update-reproducible-reqs
if dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.