-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
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.