-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use Env for AWS region, fix 2.8 deprecation warnings #38
Conversation
@fbladilo FYI, this may affect CI wrt to using the environment variable for region. |
Addresses #37 |
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 Jason!
You're welcome. I'd just like to give Franco and @Danil-Grigorev a chance to see it before merging so we don't break CI on them unexpectedly. |
Yeah, that's why I didn't want to merge it right away. I'm certainly not
blocked by this, just thought it could be an improvement.
…On Thu, May 23, 2019 at 12:29 PM Jason Montleon ***@***.***> wrote:
You're welcome. I'd just like to give Franco and @Danil-Grigorev
<https://github.com/Danil-Grigorev> a chance to see it before merging so
we don't break CI on them unexpectedly.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#38?email_source=notifications&email_token=AADEKNLC3NQRT7L5LP6DSODPW3A4VA5CNFSM4HPGK5LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWCYS3I#issuecomment-495290733>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADEKNLFDLVC4YA3K22SCJLPW3A4VANCNFSM4HPGK5LA>
.
|
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.
LGTM will update CI pipeline scripts to pass AWS_REGION
Cool. Sounds good. I'll wait until at least Monday so you have a chance and then merge. |
No description provided.