thiru-mg commented on code in PR #3600: URL: https://github.com/apache/avro/pull/3600#discussion_r2629595120
########## lang/c++/include/avro/Config.hh: ########## @@ -21,17 +21,29 @@ // Windows DLL support -#ifdef _WIN32 +#ifdef _MSC_VER #pragma warning(disable : 4275 4251) +#endif // _MSC_VER + +#if defined _WIN32 || defined __CYGWIN__ Review Comment: We are introducing three new macros (in addition to the existing `AVRO_DECL`). Do we intend to use them within Avro library or expect the users of the library to use them? (`AVRO_DECL` is meant to be used only in headers in the library). I suppose the answer is `no` for both. Since macros do not respect namespace or scoping, they pollute the symbol space. Is it better, for example, to directly define `AVRO_DECL` directly in terms of `__declspec` or `gnu::visibility`? Or, at least, let's comment the macros on their purpose and usage. -- 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]
