Skip to content

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

Merged
davidbarsky merged 1 commit into
aws:masterfrom
khuey:downstream
Oct 27, 2020
Merged

When constructing a Context from a HeaderMap, fill in the env_config field#248
davidbarsky merged 1 commit into
aws:masterfrom
khuey:downstream

Conversation

@khuey

@khuey khuey commented Aug 4, 2020

Copy link
Copy Markdown
Contributor

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

rimutaka commented Aug 4, 2020

Copy link
Copy Markdown
Contributor

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

khuey commented Aug 4, 2020

Copy link
Copy Markdown
Contributor Author

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 :)

@khuey

khuey commented Sep 8, 2020

Copy link
Copy Markdown
Contributor Author

Ping?

@davidbarsky

davidbarsky commented Sep 12, 2020

Copy link
Copy Markdown
Contributor

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

Comment thread lambda/src/types.rs Outdated

@davidbarsky davidbarsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@khuey

khuey commented Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

Anybody going to merge this?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants