You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by PivotalSarge <gi...@git.apache.org> on 2017/03/09 20:27:56 UTC

[GitHub] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...

GitHub user PivotalSarge opened a pull request:

    https://github.com/apache/geode-native/pull/50

    GEODE-2636: Switch to CMake variable for library name.

    - Parameterize the name of the library for which the quickstarts look.
    - Fix warnings that obfuscate when the quickstarts can not find a library.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2636

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/geode-native/pull/50.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #50
    
----
commit e897dd59a4f03eb22483bb41b8db38bb84686132
Author: Sarge <md...@pivotal.io>
Date:   2017-03-08T22:28:52Z

    GEODE-2636: Switch to CMake variable for library name.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...

Posted by PivotalSarge <gi...@git.apache.org>.
Github user PivotalSarge commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/50#discussion_r105266335
  
    --- Diff: src/quickstart/cpp/PdxSerializer.cpp ---
    @@ -165,15 +165,15 @@ int main(int argc, char** argv) {
         LOGINFO("Registered Person Query Objects");
     
         // Populate the Region with some Person objects.
    -    Person* p1 = new Person("John", 1 /*ID*/, 23 /*age*/);
    +    Person* p1 = new Person((char *)"John", 1 /*ID*/, 23 /*age*/);
    --- End diff --
    
    Those methods pass a string literal to a method whose first parameter is char *. An alternative would be to modify the type of the parameter but that was a more invasive change when the goal was to reduce the clutter in the log that got in the way of determining the root cause.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...

Posted by echobravopapa <gi...@git.apache.org>.
Github user echobravopapa commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/50#discussion_r105265184
  
    --- Diff: src/quickstart/cpp/PdxSerializer.cpp ---
    @@ -165,15 +165,15 @@ int main(int argc, char** argv) {
         LOGINFO("Registered Person Query Objects");
     
         // Populate the Region with some Person objects.
    -    Person* p1 = new Person("John", 1 /*ID*/, 23 /*age*/);
    +    Person* p1 = new Person((char *)"John", 1 /*ID*/, 23 /*age*/);
    --- End diff --
    
    What led to adding this cast?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...

Posted by metatype <gi...@git.apache.org>.
Github user metatype commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/50#discussion_r105272239
  
    --- Diff: src/quickstart/cpp/CMakeLists.txt ---
    @@ -26,6 +26,9 @@ elseif(UNIX)
         set(DYNAMIC_LIBRARY_PATH LD_LIBRARY_PATH=${NATIVECLIENT_DIR}/lib)
     endif()
     
    +set(PRODUCT_LIB_NAME "apache-geode" CACHE STRING "Binary name")
    --- End diff --
    
    Ah, makes sense.  I was wondering how that got set.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/geode-native/pull/50


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---