Skip to content

test: adding tests for set-cookie being handled correctly#7541

Merged
alyssawilk merged 3 commits intoenvoyproxy:masterfrom
alyssawilk:cookie
Jul 16, 2019
Merged

test: adding tests for set-cookie being handled correctly#7541
alyssawilk merged 3 commits intoenvoyproxy:masterfrom
alyssawilk:cookie

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

Yay, we're doing it right!

Risk Level: n/a (test only)
Testing: yes
Docs Changes: no
Release Notes: no
#7488

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
PiotrSikora
PiotrSikora previously approved these changes Jul 11, 2019
Copy link
Copy Markdown
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

LGTM, one small nit.

public:
enum class HeaderMatchType { Value, Regex, Range, Present, Prefix, Suffix };

// Get all instances of the header key specified, and return the values in the
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.

Nit: Could you turn this into

/*
 * Get all instances of the header key specified.
 * @param ...
 * @param ...
 */

style comment and move it after struct HeaderData?

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Copy Markdown
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Copy Markdown
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks!

@alyssawilk alyssawilk merged commit 29a1620 into envoyproxy:master Jul 16, 2019
@alyssawilk alyssawilk deleted the cookie branch July 31, 2019 20:33
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