Skip to content

build: declare original IP detection factories#16621

Merged
snowp merged 1 commit intoenvoyproxy:mainfrom
goaway:ms/declare-ip-detect-factories
May 21, 2021
Merged

build: declare original IP detection factories#16621
snowp merged 1 commit intoenvoyproxy:mainfrom
goaway:ms/declare-ip-detect-factories

Conversation

@goaway
Copy link
Copy Markdown
Contributor

@goaway goaway commented May 21, 2021

Commit Message: build: declare original IP detection factories
Risk Level: Low
Testing: Local & CI

Signed-off-by: Mike Schore mike.schore@gmail.com

Signed-off-by: Mike Schore <mike.schore@gmail.com>
@goaway
Copy link
Copy Markdown
Contributor Author

goaway commented May 21, 2021

cc @phlax

goaway added a commit to phlax/envoy-mobile that referenced this pull request May 21, 2021
Signed-off-by: Mike Schore <mike.schore@gmail.com>
@phlax
Copy link
Copy Markdown
Member

phlax commented May 21, 2021

cc @rgs1

Copy link
Copy Markdown
Member

@rgs1 rgs1 left a comment

Choose a reason for hiding this comment

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

Is this something that can be checked by CI to avoid future factories from missing this? Thanks for the fix.

@goaway
Copy link
Copy Markdown
Contributor Author

goaway commented May 21, 2021

@rgs In theory one could probably introduce a dependency between the definitions of REGISTER_FACTORY and DECLARE_FACTORY that would cause the former to fail statically when the latter is missing.

Copy link
Copy Markdown
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

Thanks!

@snowp snowp merged commit db0a477 into envoyproxy:main May 21, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Signed-off-by: Mike Schore <mike.schore@gmail.com>
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.

4 participants