Skip to content

When constructing a Context from a HeaderMap, fill in the env_config field #248

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 27, 2020
Merged

When constructing a Context from a HeaderMap, fill in the env_config field #248

merged 1 commit into from
Oct 27, 2020

Conversation

khuey
Copy link
Contributor

@khuey khuey commented Aug 4, 2020

Description of changes: Currently this just produces a useless env_config.

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

@rimutaka
Copy link
Contributor

rimutaka commented Aug 4, 2020

Good spotting!

We also missed pub client_context: Option<ClientContext> and pub identity: Option<CognitoIdentity> in the same Context structure.
They are declared, but are never used. I'll need both of them in a few weeks, so if no one fixes it by then I'll have to :)

@khuey
Copy link
Contributor Author

khuey commented Aug 4, 2020

Doesn't look like ClientContext or CognitoIdentity have any existing from_the_relevant_thing methods though, so I'd prefer just to merge this as is :)

@rimutaka rimutaka mentioned this pull request Aug 4, 2020
Closed
@khuey
Copy link
Contributor Author

khuey commented Sep 8, 2020

Ping?

@davidbarsky
Copy link
Contributor

davidbarsky commented Sep 12, 2020 •
edited
Loading

Apologies for the delay, @khuey. I spoke too soon in saying that I'm happy to merge this; just one comment though.

davidbarsky
davidbarsky suggested changes Sep 12, 2020
View reviewed changes
davidbarsky
davidbarsky approved these changes Oct 10, 2020
View reviewed changes
Copy link
Contributor

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

Thanks!

@khuey
Copy link
Contributor Author

khuey commented Oct 27, 2020

Anybody going to merge this?

@davidbarsky davidbarsky merged commit 13aa8f0 into awslabs:master Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@khuey @rimutaka @davidbarsky