Conversation
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
Looks good modulo figuring out the libtooling BUILD file.
/wait
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
stale
bot
removed
the
stale
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
|
@htuch should be ready to go |
|
Can you check CI? /wait |
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
|
See changes to source/ and test/ (ignore the others) from this specific patch revert, I suspect you want a number of these changes on this PR, Sunjay has pulled them from the RBE work... d34894a |
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
| const char* cdata = reinterpret_cast<const char*>(stat_name.data()); | ||
| absl::string_view data_as_string_view = absl::string_view(cdata, stat_name.dataSize()); | ||
| return H::combine(std::move(h), data_as_string_view); | ||
| return H::combine(std::move(h), stat_name.dataAsStringView()); |
test/run_envoy_bazel_coverage.sh
Outdated
| COVERAGE_THRESHOLD=27.0 | ||
| else | ||
| COVERAGE_THRESHOLD=97.0 | ||
| COVERAGE_THRESHOLD=96.6 |
There was a problem hiding this comment.
re: coverage threshold, it is because LLVM 10.0 generate trace differently for default: and empty lines, e.g.:
There was a problem hiding this comment.
If it's purely an artifact of measurement, I'm OK with adjusting the limit, but it would be good to get this heading North.
There was a problem hiding this comment.
yeah now it's easier for us to actually exclude lines in coverage with some post processing, as lcov format is much easier to work with, but it's beyond the scope of this PR
|
@htuch finally all CI passed, PTAL |
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
LGTM modulo nghttp2 patch thread.
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
This reverts commit 74bcb11. Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
|
Reviewing status this morning of the "cancelled" coverage run, this PR is passing all tests; sent 62,518,874 bytes received 92,045 bytes 41,740,612.67 bytes/sec Our work on the Windows CI in 11107 and work to evaluate LLVM clang-cl 10 as an alternative Windows compiler are all gated on this PR, if it could be merged promptly. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s), but failed to run 2 pipeline(s). |
|
@wrowe I should be able to merge once CI pass |
Risk Level: Med Testing: CI Docs Changes: Changed clang references Release Notes: Added Signed-off-by: Lizan Zhou <lizan@tetrate.io> Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Risk Level: Med Testing: CI Docs Changes: Changed clang references Release Notes: Added Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Risk Level: Med Testing: CI Docs Changes: Changed clang references Release Notes: Added Signed-off-by: Lizan Zhou <lizan@tetrate.io> Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
6 participants
Risk Level: Med
Testing: CI
Docs Changes: Changed clang references
Release Notes: Added
Signed-off-by: Lizan Zhou lizan@tetrate.io