-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "bazel: update to 6.0.0rc1 (#23678)" #24093
Conversation
This reverts commit 84df237. Signed-off-by: Ryan Northey <ryan@synca.io>
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to |
fixing the coverage here exposes that the coverage has not been checked recently and is therefore not matching current per-file expectations i was about to adjust the coverage when i noticed that one is off by a lot Code coverage for source/extensions/clusters/common is lower than limit of 96.6 (68.2)
Code coverage for source/extensions/filters/http/cache/simple_http_cache is lower than limit of 96.0 (95.9)
Code coverage for source/extensions/filters/network/thrift_proxy/filters/payload_to_metadata is lower than limit of 96.6 (96.2)
Code coverage for source/server/admin is lower than limit of 97.5 (97.4)
|
cc @alyssawilk |
cc @keith |
This PR is marked as draft. Are you still working on it, or is it ready for review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine to merge and I can debug after the holidays
it was - i think i already deWIPPed it
it needs the per-file coverage to be adjusted - i can do that but in the case of the clusters code this is a big drop Code coverage for source/extensions/clusters/common is lower than limit of 96.6 (68.2) i guess we want to just adjust coverage and fix that separately - ill update |
/lgtm deps |
Signed-off-by: Ryan Northey <ryan@synca.io>
I think the solve for this might be this flag: --experimental_fetch_all_coverage_outputs |
let me give that a whirl on a test PR ... |
Guess we should also make this stuff fail when it breaks |
that doesnt seem to have worked -r-xr-xr-x 1 envoybuild envoygroup 0 Nov 21 18:07 **_coverage_report.dat** |
i reckon lets land this for now and address the bazel/update/coverage issue separately |
dont-break-coverage ticket is here #24119 |
missing-coverage ticket #24120 |
- Update the ENVOY_COMMIT and ENVOY_SHA in [bazel/repositories.bzl](https://github.com/envoyproxy/nighthawk/blob/main/bazel/repositories.bzl) to the latest Envoy's commit. - Update [.bazelrc](https://github.com/envoyproxy/nighthawk/blob/main/.bazelrc) to match Envoy commit envoyproxy/envoy#24093 - Update [.bazelversion](https://github.com/envoyproxy/nighthawk/blob/main/.bazelversion) to match Envoy commit envoyproxy/envoy#24093 - Update [ci/run_envoy_docker.sh](https://github.com/envoyproxy/nighthawk/blob/main/ci/run_envoy_docker.sh) to match Envoy commit envoyproxy/envoy#24107 Signed-off-by: tomjzzhang <4367421+tomjzzhang@users.noreply.github.com>
Fix #23985
This reverts commit 84df237.
Signed-off-by: Ryan Northey ryan@synca.io
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:]