Skip to content
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

Small fixes for macos / clang builds #135

Merged
merged 2 commits into from
May 13, 2020
Merged

Small fixes for macos / clang builds #135

merged 2 commits into from
May 13, 2020

Conversation

cgilmour
Copy link
Contributor

Under some environments, std::chrono::duration uses long for representing the value, and others use long long. In the latter, it caused an error comparing against a long using std::min.
It could have been fixed using std::common_type, but a simpler fix with a basic if comparison was chosen instead.

When running tests on macos, one specific test would fail in a mysterious way, when comparing against an essentially static value. This was somehow due to the orders libraries were linked causing trouble on this environment. (It didn't occur with Linux/GCC).

cgilmour added 2 commits May 11, 2020 11:29
Tests were not passing under macos in mysterious ways - a reference to a
static error_category reference were not behaving correctly.
The underlying cause was linking dd_opentracing after the opentracing
library. It'd essentially call a different version of the function and
get a different (static) error_category reference.
@cgilmour cgilmour requested a review from brettlangdon May 11, 2020 02:58
@cgilmour
Copy link
Contributor Author

These came up as part of envoyproxy/envoy#11094, which failed some of its CI checks. We need a new release before reopening a PR for that project.

@cgilmour cgilmour merged commit 752c666 into master May 13, 2020
@cgilmour cgilmour deleted the cgilmour/macos-fixes branch May 13, 2020 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants