-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix namespace resolution issue in LOG_EVERY_T #801
Conversation
Codecov Report
@@ Coverage Diff @@
## master #801 +/- ##
=======================================
Coverage 73.20% 73.20%
=======================================
Files 17 17
Lines 3281 3281
=======================================
Hits 2402 2402
Misses 879 879
Continue to review full report at Codecov.
|
f877f25 to
6cc721c
Compare
|
I stuggle to see the link between my change and the failing tests on MacOS. |
|
Thanks for the PR. The macOS runner seems to be volatile or our tests flaky. Can you provide a reproducer for the failures you are describing? I'm just wondering why our CI did not catch these problems. |
|
I honestly don't understand either, I checked and this part of the code seem covered by the tests. I'll try to create a sample project that reproduce it. Meanwhile I can at least describe our setup. |
|
Thanks for the details. @drigz It seems we need a Bazel runner that uses |
This should catch issues like #801. This uses the new `tasks` syntax to define multiple Windows tasks: https://github.com/bazelbuild/continuous-integration/blob/master/buildkite/README.md#basic-syntax
This should catch issues like #801. This uses the new `tasks` syntax to define multiple Windows tasks: https://github.com/bazelbuild/continuous-integration/blob/master/buildkite/README.md#basic-syntax
This should catch issues like #801. This uses the new `tasks` syntax to define multiple Windows tasks: https://github.com/bazelbuild/continuous-integration/blob/master/buildkite/README.md#basic-syntax
This should catch issues like #801. This uses the new `tasks` syntax to define multiple Windows tasks: https://github.com/bazelbuild/continuous-integration/blob/master/buildkite/README.md#basic-syntax
|
@skeptic-monkey Could you please rebase? |
Done. |
|
Thanks! |
int64 is a typedef defined within @ac_google_namespace@, while LOG_EVERY_T is defined within ::google:: namespace.
but @ac_google_namespace@ is configurable and might not always be "google".
It can result in a compilation error, int64 being undefined. using absolute namespace resolution with @ac_google_namespace@:: fixes it.
Also replaced 2 other hardcoded google:: usage.