You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/09/01 18:48:28 UTC

[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #645: GEODE-8469: Enable no-missing-variable-declarations

pivotal-jbarrett commented on a change in pull request #645:
URL: https://github.com/apache/geode-native/pull/645#discussion_r481354457



##########
File path: cppcache/benchmark/GeodeLoggingBM.cpp
##########
@@ -106,13 +106,13 @@ void GeodeLogToFile(benchmark::State& state) {
   }
 }
 
-auto LogStringsToConsole = GeodeLogToConsole<GeodeLogStrings>;
-auto LogIntsToConsole = GeodeLogToConsole<GeodeLogInts>;
-auto LogComboToConsole = GeodeLogToConsole<GeodeLogCombo>;
+static auto LogStringsToConsole = GeodeLogToConsole<GeodeLogStrings>;

Review comment:
       `const` also?

##########
File path: tests/cpp/security/XmlAuthzCredentialGenerator.hpp
##########
@@ -55,7 +55,7 @@ const opCodeList::value_type QArr[] = {OP_QUERY, OP_REGISTER_CQ};
 
 const stringList::value_type QRArr[] = {"Portfolios", "Positions"};
 
-const char* PRiUsnm = "%s%d";
+static const char* PRiUsnm = "%s%d";

Review comment:
       `constexpr` work here?

##########
File path: cppcache/integration-test/fw_dunit.cpp
##########
@@ -71,10 +71,10 @@ using apache::geode::client::testframework::BBNamingContextServer;
 #define __DUNIT_NO_MAIN__
 #include "fw_dunit.hpp"
 
-ACE_TCHAR *g_programName = nullptr;
-uint32_t g_coordinatorPid = 0;
+static ACE_TCHAR *g_programName = nullptr;

Review comment:
       Yuck! I attempted to take on this beast and gave up on trying to clean up the old tests. Good job!

##########
File path: cppcache/test/CacheXmlParserTest.cpp
##########
@@ -99,7 +99,7 @@ std::string valid_cache_config_body = R"(<region name = 'Root1' >
     </region>
 </client-cache>)";
 
-std::string invalid_cache_config_body = R"(<region >
+static std::string invalid_cache_config_body = R"(<region >

Review comment:
       `const` ?

##########
File path: cppcache/src/PdxFieldType.cpp
##########
@@ -31,7 +31,7 @@ namespace apache {
 namespace geode {
 namespace client {
 
-int32_t fixedTypeSizes[] = {
+static int32_t fixedTypeSizes[] = {

Review comment:
       `const` and use the `const` naming convention here?




----------------------------------------------------------------
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:
users@infra.apache.org