[ https://issues.apache.org/jira/browse/GEODE-8921?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17279854#comment-17279854 ]
ASF GitHub Bot commented on GEODE-8921: --------------------------------------- moleske commented on a change in pull request #741: URL: https://github.com/apache/geode-native/pull/741#discussion_r571142016 ########## File path: CMakeLists.txt ########## @@ -281,6 +281,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") target_compile_options(_WarningsAsError INTERFACE /WX /wd4996 + /we4596 Review comment: I was going to write indentation, but I actually am coming around to indenting every line with different indentation ########## File path: cppcache/integration/benchmark/RegionBM.cpp ########## @@ -19,6 +19,17 @@ #include <framework/Cluster.h> #include <framework/Gfsh.h> +// Disable warning for "extra qualifications" here. One of the boost log +// headers triggers this warning. Note: use of disable pragma here is +// intentional - attempts to use push/pop as you ordinarily should just +// yielded a gripe from the MS tools that "warning number '4596' is not a +// valid compiler warning". re-enabling the warning after the include +// fails in the same way, so just leave it disabled for the rest of the +// file. This is safe, since the warning can only trigger inside a class Review comment: There's a class defined on line 51? Or did you mean something else by this comment? ########## File path: clicache/src/impl/PdxHelper.hpp ########## @@ -46,15 +46,15 @@ namespace Apache static IPdxSerializable^ DeserializePdx(DataInput^ dataOutput, bool forceDeserialize, const native::SerializationRegistry* serializationRegistry); - static IPdxSerializable^ PdxHelper::DeserializePdx(DataInput^ dataInput, bool forceDeserialize, int typeId, int length, const native::SerializationRegistry* serializationRegistry); + static IPdxSerializable^ DeserializePdx(DataInput^ dataInput, bool forceDeserialize, int typeId, int length, const native::SerializationRegistry* serializationRegistry); literal Byte PdxHeader = 8; static Int32 ReadInt32(System::Byte* offsetPosition); static Int32 ReadInt16(System::Byte* offsetPosition); - static Int32 PdxHelper::ReadUInt16(System::Byte* offsetPosition); + static Int32 ReadUInt16(System::Byte* offsetPosition); Review comment: Still weird indentation if you decide it's worth it ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Enable "extra qualifications" warning on Windows builds > ------------------------------------------------------- > > Key: GEODE-8921 > URL: https://issues.apache.org/jira/browse/GEODE-8921 > Project: Geode > Issue Type: Improvement > Components: native client > Reporter: Blake Bender > Assignee: Blake Bender > Priority: Major > Labels: pull-request-available > > As a developer, I would like to be assured as much as possible that, if my > code builds on the platform I'm running on my dev machine, it will build for > all of our supported platforms. One very notable exception to this has been > the "extra qualifications" warning, which is not enabled by default in Visual > Studio. Thus, the following code will build in our environment on Windows, > but _only_ on Windows: > {code:java} > class Foo > { > public: > void Foo::bar(); > }; {code} > This is warning C4596 in the Microsoft tools, and we need to explicitly > enable it in our builds. -- This message was sent by Atlassian Jira (v8.3.4#803005)