Skip to content

test: This constructor no longer makes sense in the context of the global test-scoped singleton introduced in https://github.com/envoyproxy/envoy/pull/5708#79

Merged
dnoe merged 1 commit intoenvoyproxy:masterfrom
jmarantz:revert-three-arg-ctor
Mar 1, 2019
Merged

test: This constructor no longer makes sense in the context of the global test-scoped singleton introduced in https://github.com/envoyproxy/envoy/pull/5708#79
dnoe merged 1 commit intoenvoyproxy:masterfrom
jmarantz:revert-three-arg-ctor

Conversation

@jmarantz
Copy link
Contributor

@jmarantz jmarantz commented Mar 1, 2019

This constructor no longer makes sense in the context of the global test-scoped singleton introduced in envoyproxy/envoy#5708 .

…est-scoped singleton introduced in envoyproxy/envoy#5708

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Contributor

@dnoe dnoe left a comment

Choose a reason for hiding this comment

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

LGTM. If we merge this now, does it mean you can remove the TODOs from envoyproxy/envoy#6139 ?

@jmarantz
Copy link
Contributor Author

jmarantz commented Mar 1, 2019

I think I need to do something about a sha update, but I forgot whether that's in this repo or the main one.

In the meantime the two PRs can be checked in independently, fortunately.

@dnoe
Copy link
Contributor

dnoe commented Mar 1, 2019

This repo automagically updates the submodule SHA with each merged PR in the main (see c12d58f for an example of one of the robo commits that does this). So there's no manual action required.

I'm going to merge this one now since they are independent. Then I will review your main repo PR. I think you can now update it to remove the bits required before this was merged.

@dnoe dnoe merged commit de52a19 into envoyproxy:master Mar 1, 2019
@jmarantz jmarantz deleted the revert-three-arg-ctor branch March 1, 2019 16:57
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.

2 participants