evetion commented on code in PR #2649: URL: https://github.com/apache/thrift/pull/2649#discussion_r967848189
########## lib/cpp/src/thrift/transport/TServerSocket.cpp: ########## @@ -70,7 +70,7 @@ // adds problematic macros like min() and max(). Try to work around: #define NOMINMAX #define WIN32_LEAN_AND_MEAN -#include <Windows.h> +#include <windows.h> Review Comment: > Based on Stackoverflow, we assume that newer VC++ installations and Windows SDKs seem to use Windows.h (with uppercase first letter), correct? Yes. > If I where to copy or extract such an installation onto a case-sensitive file system, then I would need to use Windows.h as the correct casing, correct? If you *could* copy/install such an installation yes, but one can't. So that's why in cross-compilation you'd use MinGW, which comes with their own lower-cased headers, such as <windows.h>. > And also, if Windows.h is the default casing, why does your case-sensitive installation use windows.h instead? Isn't this the "odd" case, and Windows.h is the common case? I agree that my case is the "odd" case here, especially if you're not used to cross-compiling. That said, I think supporting cross-compilation is great for open-source software and the requested change is small and doesn't impact anything (the CI errors should be unrelated). edit: The codebase does contain `defined(__MINGW32__)` statements, so it seems to support cross-compiling by design. As I'm not an expert in C++ or even cross-compilation, it might be best for the authors of the previous related [PR](https://github.com/apache/thrift/pull/2518) to chime in @jeremiahpslewis @Jens-G. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
