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

feat(cpp-client): Add GetTidAsString as a utility function #5843

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

kosak
Copy link
Contributor

@kosak kosak commented Jul 24, 2024

Add GetTidAsString() as a utility function.

Note that the previous version of this code was inline. However, on the Windows platform, making it inline involves pulling in some windows includes including windows.h. The problem with including windows.h is that it defines some macros that might conflict. In particular it defines the min and max macros that definitely conflict.

In order to free ourselves and our Windows customers from seeing the conflicts that might arise, we isolate the Windows-specific code in a .cc file rather than including windows.h in a .h file.

There are other approaches that would work too, such as defining NOMINMAX or changing all of our calls that look like min() and max() to have extra parentheses: (min)() and (max)().

However in my opinion, isolating this Windows-specific code in one .cc file was simplest for now.

@kosak kosak added NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Jul 24, 2024
@kosak kosak requested a review from jcferretti July 24, 2024 22:58
@kosak kosak self-assigned this Jul 24, 2024
jcferretti
jcferretti previously approved these changes Jul 24, 2024
@kosak kosak merged commit 8a7329e into deephaven:main Jul 24, 2024
15 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2024
@kosak kosak deleted the kosak_get-thread-id branch July 24, 2024 23:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants