[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user asfgit closed the pull request at: https://github.com/apache/geode-native/pull/13 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: [GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
I've modified PR #13 to remove the vestigial executable and to elide the duplicate GEODE_ in the legacy include guards. Sarge > On 17 Feb, 2017, at 11:06, pivotal-jbarrett wrote: > > Github user pivotal-jbarrett commented on a diff in the pull request: > >https://github.com/apache/geode-native/pull/13#discussion_r101825168 > >--- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- >@@ -1,7 +1,7 @@ > #pragma once > >-#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ >-#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ >+#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ >--- End diff -- > >I see an argument that `include/geode/` is the root for the public > headers, therefore the guards should be `GEODE_ATTRIBUTESFACTORY_H_`. > > > --- > 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 infrastruct...@apache.org or file a JIRA ticket > with INFRA. > ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101825168 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -1,7 +1,7 @@ #pragma once -#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ -#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ +#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ --- End diff -- I see an argument that `include/geode/` is the root for the public headers, therefore the guards should be `GEODE_ATTRIBUTESFACTORY_H_`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101824812 --- Diff: src/CMakeLists.txt --- @@ -224,7 +222,7 @@ add_subdirectory(cppcache) add_subdirectory(cryptoimpl) add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) -add_subdirectory(gfcpp) +add_subdirectory(getversion) --- End diff -- If they are relying on it then their scripts will break either way. There are better ways to get version. I don't think I have ever seen anyone anywhere use it. I don't even see it in our documents. I say kill it! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101796694 --- Diff: src/CMakeLists.txt --- @@ -224,7 +222,7 @@ add_subdirectory(cppcache) add_subdirectory(cryptoimpl) add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) -add_subdirectory(gfcpp) +add_subdirectory(getversion) --- End diff -- I'm totally cool with deleting it. I didn't because it's been part of the install; some users may rely on it to determine version? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101796462 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -1,7 +1,7 @@ #pragma once -#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ -#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ +#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ --- End diff -- Google specification is project name, directory name, file 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101796329 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -20,7 +20,7 @@ * limitations under the License. */ -#include "gfcpp_globals.hpp" +#include "geode_globals.hpp" #include "gf_types.hpp" --- End diff -- That's a separate JIRA ticket and hence will be a separate (upcoming) PR. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user mmartell commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101783052 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -20,7 +20,7 @@ * limitations under the License. */ -#include "gfcpp_globals.hpp" +#include "geode_globals.hpp" #include "gf_types.hpp" --- End diff -- +1 for geode_types.hpp --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user mmartell commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101782908 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -1,7 +1,7 @@ #pragma once -#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ -#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ +#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ --- End diff -- +1 for GEODE_ATTRIBUTES_FACTORY_H_ --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101659366 --- Diff: src/CMakeLists.txt --- @@ -224,7 +222,7 @@ add_subdirectory(cppcache) add_subdirectory(cryptoimpl) add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) -add_subdirectory(gfcpp) +add_subdirectory(getversion) --- End diff -- Eeeek! This is the best name we could come up with for this stupid util? May we should just delete it? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101659601 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -1,7 +1,7 @@ #pragma once -#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ -#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ +#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ --- End diff -- Really? GEODE_GEODE doesn't seem right?! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101659693 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -20,7 +20,7 @@ * limitations under the License. */ -#include "gfcpp_globals.hpp" +#include "geode_globals.hpp" #include "gf_types.hpp" --- End diff -- Why not gf_types.hpp renamed to geode_types.hpp? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/13 GEODE-2476: Replace gfcpp with geode. - Rename directories and files with gfcpp into their name to instead use geode and update all references thereto. - Rename the gfcpp executable to apache-geode-getversion and modify it to print the version even in the absence of command-line arguments. - Rename gfcpp.properties to geode.properties and update all references thereto. - Ensure formatting style guide compliance. - Re-applying fixes for Windows compilation errors. - Fix logic for using clang tidy auto-fix. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2476 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/13.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 #13 commit 56e21e5e89bed28416237c6d752d60f9d894e9b7 Author: Sarge Date: 2017-02-14T18:31:51Z GEODE-2476: Replace gfcpp with geode. - Rename directories and files with gfcpp into their name to instead use geode and update all references thereto. - Rename the gfcpp executable to apache-geode-getversion and modify it to print the version even in the absence of command-line arguments. - Rename gfcpp.properties to geode.properties and update all references thereto. - Ensure formatting style guide compliance. - Re-applying fixes for Windows compilation errors. - Fix logic for using clang tidy auto-fix. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
