Skip to content

mobile: attempting to remove more of protoc#24151

Closed
alyssawilk wants to merge 4 commits intoenvoyproxy:mainfrom
alyssawilk:protoc
Closed

mobile: attempting to remove more of protoc#24151
alyssawilk wants to merge 4 commits intoenvoyproxy:mainfrom
alyssawilk:protoc

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

Signed-off-by: Alyssa Wilk alyssar@chromium.org

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #24151 was opened by alyssawilk.

see: more, trace.

@repokitteh-read-only repokitteh-read-only Bot added the deps Approval required for changes to Envoy's external dependencies label Nov 22, 2022
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #24151 was opened by alyssawilk.

see: more, trace.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk added a commit to envoyproxy/envoy-mobile that referenced this pull request Nov 23, 2022
This fixes up issues exposed in envoyproxy/envoy#24151 where the PlatformBridgeCertValidatorFactory was not associated with the PlatformBridgeCertValidator proto.

Risk Level: low
Testing: envoyproxy/envoy#24151
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk added a commit that referenced this pull request Nov 23, 2022
Currently proto_gen_validate pulls all protos into every unit test, and every unit test validates their existence on start up. This is pretty terrible. bufbuild/protoc-gen-validate#738 pulls this out for Envoy Mobile proto-gen-validate-less builds, and so breaks all the unit tests. Changing the Envoy test framework to only validate proto registration for integration tests.

Risk Level: low (should be test only)
Testing: #24151
Docs Changes: n/a
Release Notes: n/a
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk closed this Nov 23, 2022
elliotmjackson pushed a commit to bufbuild/protoc-gen-validate that referenced this pull request Nov 28, 2022
before
-rwxr-x--- 1 alyssar primarygroup 23338512 Nov 21 17:00
bazel-bin/test/performance/test_binary_size_stripped
-rwxr-x--- 1 alyssar primarygroup 23M Nov 21 17:00
bazel-bin/test/performance/test_binary_size_stripped
after
-rwxr-x--- 1 alyssar primarygroup 19642512 Nov 21 17:04
bazel-bin/test/performance/test_binary_size_stripped
-rwxr-x--- 1 alyssar primarygroup 19M Nov 21 17:04
bazel-bin/test/performance/test_binary_size_stripped

Proof of concept use here:
envoyproxy/envoy#24151 which is passing all
Envoy Mobile CI (its failing Envoy CI because of the dependency check
which is expected and irrelevant)
jpsim pushed a commit that referenced this pull request Nov 29, 2022
This fixes up issues exposed in #24151 where the PlatformBridgeCertValidatorFactory was not associated with the PlatformBridgeCertValidator proto.

Risk Level: low
Testing: #24151
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants