GEODE-4343: Add auto-serialization example for .net #227

pdxcodemonkey · 2018-02-23T21:07:21Z

Signed-off-by: Ivan Godwin igodwin@pivotal.io
Signed-off-by: Ryan McMahon rmcmahon@pivotal.io

jake-at-work · 2018-02-23T21:09:20Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+cmake_minimum_required(VERSION 3.8.2)


3.10 is required to do CSharp.

jake-at-work · 2018-02-23T21:09:34Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+
+project(AuthInitialize CSharp)
+
+message(STATUS "Assembly is ${GEODE_ASSEMBLY}")


Debug messages?

jake-at-work · 2018-02-23T21:09:51Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+
+add_executable(${PROJECT_NAME} 
+    Program.cs
+	ExampleAuthInitialize.cs)


jake-at-work · 2018-02-23T21:10:28Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+message(STATUS "project binary dir: ${PROJECT_BINARY_DIR}")
+
+message(STATUS "current source dir is ${CMAKE_CURRENT_SOURCE_DIR}")
+message(STATUS "Attempting to link to ${CMAKE_CURRENT_SOURCE_DIR}/../../../build/clicache/src/Debug/Apache.Geode.dll")


Hardcoded build directory!

jake-at-work · 2018-02-23T21:10:36Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+    Program.cs
+	ExampleAuthInitialize.cs)
+
+message(STATUS "output dir is ${RUNTIME_OUTPUT_DIRECTORY}")


Debug output?

jake-at-work

The checked in .csproj files should be deleted now that a CMake file has been added.

jake-at-work · 2018-02-23T21:11:50Z

examples/dotnet/AuthInitialize/startserver.ps1

+{
+    if (-not (Test-Path env:GEODE_HOME))
+    {
+        echo "Could not find gfsh.  Please set the GEODE_HOME path. e.g. "


Why write powershell script and use cmd echo?

jake-at-work · 2018-02-23T21:13:19Z

examples/dotnet/CMakeLists.txt

@@ -16,4 +16,6 @@ cmake_minimum_required(VERSION 3.4)

 project(Apache.Geode.Examples)


Needs language NONE

jake-at-work · 2018-02-23T21:13:57Z

examples/dotnet/PdxAutoSerializer/CMakeLists.txt

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+cmake_minimum_required(VERSION 3.8.2)


CSharp requires 3.10.

Also, below all the same comments that the other CMake file has.

jake-at-work · 2018-02-23T21:18:18Z

examples/dotnet/PdxAutoSerializer/Order.cs

+            return "Order: [" + OrderId + ", " + Name + ", " + Quantity + "]";
+        }
+    }
+}


Should end with a blank line.

What the heck??? I definitely added these, looks like VS or maybe Git "helped" me by taking the newline back out. Will try again.

jake-at-work · 2018-02-23T21:18:27Z

examples/dotnet/PdxAutoSerializer/Program.cs

+            cache.Close();
+        }
+    }
+}


Should end with a blank line.

jake-at-work · 2018-02-23T21:18:51Z

examples/dotnet/PdxAutoSerializer/startserver.ps1

+{
+    if (-not (Test-Path env:GEODE_HOME))
+    {
+        echo "Could not find gfsh.  Please set the GEODE_HOME path. e.g. "


Same issues as with the other powershell script.

jake-at-work · 2018-02-23T21:20:08Z

examples/dotnet/PutGetRemove/Program.cs

+
+            Console.WriteLine("Storing id and username in the region");
+
+            const string rtimmonsKey = "rtimmons";


Still, why not using var since the type is deduced by the rhs?

Using const string prevents the variable from being modified once initialized.

So this won't compile:

const string foo = "foo" foo = "bar"

but this will:

var foo = "foo" foo = "bar"

Using 'const string' is mostly an indication of intent (rtimmonsKey should not be modified after initialization), but also protects against accidentally reassigning it at some point (unlikely).

jake-at-work · 2018-02-23T21:25:41Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+
+message(STATUS "Assembly is ${GEODE_ASSEMBLY}")
+
+if ("${GEODE_ASSEMBLY}" STREQUAL "")


Should be using the find_package to find this and not force the user to specify it.

The find_package call in Findgeode-native.cmake is broken and we made the decision to just require the location explicitly rather than rathole on fixing the script for this story.

jake-at-work · 2018-02-26T20:27:08Z

examples/dotnet/AuthInitialize/CMakeLists.txt

+
+project(AuthInitialize CSharp)
+
+if ("${GEODE_ASSEMBLY}" STREQUAL "")


Use find_package.

jake-at-work · 2018-02-26T20:30:55Z

examples/dotnet/AuthInitialize/startserver.ps1

+{
+    if (-not (Test-Path env:GEODE_HOME))
+    {
+        write-output "Could not find gfsh.  Please set the GEODE_HOME path. e.g. "


Write-Output to be consistent with PS formatting. May want to consider Write-Host.

jake-at-work · 2018-02-26T20:32:21Z

examples/dotnet/PutGetRemove/Program.cs

+
+            Console.WriteLine("Storing id and username in the region");
+
+            const string rtimmonsKey = "rtimmons";


Still, why not using var since the type is deduced by the rhs?

jake-at-work · 2018-02-28T16:22:56Z

@austxcodemonkey Please rebase and force push.

Signed-off-by: Ivan Godwin <igodwin@pivotal.io> Signed-off-by: Ryan McMahon <rmcmahon@pivotal.io>

jake-at-work · 2018-03-05T16:44:03Z

examples/cmake/FindGeodeNative.cmake

  if(NOT TARGET ${GEODE_NATIVE_CPP_TARGET})
+    set(GEODE_NATIVE_CPP_TARGET "apache.geode::cpp")


I think we should keep consistent naming. Maybe we should change this from apache.geode::cpp to ApacheGeode::cpp to match find_package(ApacheGeode COMPONENT cpp).

jake-at-work · 2018-03-05T16:45:00Z

examples/cmake/FindGeodeNative.cmake

+      INTERFACE_INCLUDE_DIRECTORIES "${GEODE_NATIVE_CPP_INCLUDE_DIR}")
+  endif()
+  if(NOT TARGET ${GEODE_NATIVE_DOTNET_TARGET})
+    set(GEODE_NATIVE_DOTNET_TARGET "apache.geode::dotnet")


Same here for naming.

jake-at-work · 2018-03-05T16:45:33Z

examples/cmake/FindGeodeNative.cmake

@@ -31,15 +31,14 @@
 #
 # This module will set the following variables in your project:
 #
-# ``GEODE_NATIVE_INCLUDE_DIRS``
+# ``GEODE_NATIVE_CPP_INCLUDE_DIRS``
 #   where to find CacheFactory.hpp, etc.
 # ``GEODE_NATIVE_LIBRARIES``
 #   the libraries to link against to use Geode Native.
 # ``GEODE_NATIVE_FOUND``
 #   true if the Geode Native headers and libraries were found.
 #


Need to verify there names are still valid and update the docs to include the dotnet component.

* Normalize naming convention in find module.

jake-at-work requested changes Feb 23, 2018

View reviewed changes

pdxcodemonkey changed the title ~~GEODE-4341: Add auto-serialization example for .net~~ GEODE-4343: Add auto-serialization example for .net Feb 23, 2018

jake-at-work requested changes Feb 26, 2018

View reviewed changes

GEODE-4341: Add auto-serialization example for .net

1c29165

Signed-off-by: Ivan Godwin <igodwin@pivotal.io> Signed-off-by: Ryan McMahon <rmcmahon@pivotal.io>

pdxcodemonkey force-pushed the feature/GEODE-4343 branch from e8018a1 to 1c29165 Compare March 2, 2018 00:01

jake-at-work requested changes Mar 5, 2018

View reviewed changes

Blake Bender added 2 commits March 5, 2018 10:17

GEODE-4343: Normalize naming convention in find module.

8d368f8

GEODE-4343: Fix build breaks from prev. commit

5cb6e88

jake-at-work approved these changes Mar 5, 2018

View reviewed changes

jake-at-work merged commit 8eb5c67 into apache:develop Mar 5, 2018

mmartell pushed a commit to mmartell/geode-native that referenced this pull request Mar 6, 2018

GEODE-4343: Add auto-serialization example for .net (apache#227)

3aa6657

* Normalize naming convention in find module.

mmartell pushed a commit to mmartell/geode-native that referenced this pull request Mar 6, 2018

GEODE-4343: Add auto-serialization example for .net (apache#227)

aa98be2

* Normalize naming convention in find module.

jake-at-work pushed a commit to jake-at-work/geode-native that referenced this pull request Oct 6, 2018

GEODE-4343: Add auto-serialization example for .net (apache#227)

0cebbad

* Normalize naming convention in find module.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GEODE-4343: Add auto-serialization example for .net #227

GEODE-4343: Add auto-serialization example for .net #227

pdxcodemonkey commented Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work left a comment

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

pdxcodemonkey Feb 26, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 23, 2018

jake-at-work Feb 26, 2018

mcmellawatt Mar 1, 2018

jake-at-work Feb 23, 2018

pdxcodemonkey Feb 26, 2018

jake-at-work Feb 26, 2018

jake-at-work Feb 26, 2018

jake-at-work Feb 26, 2018

jake-at-work commented Feb 28, 2018

jake-at-work Mar 5, 2018

jake-at-work Mar 5, 2018

jake-at-work Mar 5, 2018


		project(AuthInitialize CSharp)

		message(STATUS "Assembly is ${GEODE_ASSEMBLY}")

		@@ -16,4 +16,6 @@ cmake_minimum_required(VERSION 3.4)

		project(Apache.Geode.Examples)


		Console.WriteLine("Storing id and username in the region");

		const string rtimmonsKey = "rtimmons";


		message(STATUS "Assembly is ${GEODE_ASSEMBLY}")

		if ("${GEODE_ASSEMBLY}" STREQUAL "")

		if(NOT TARGET ${GEODE_NATIVE_CPP_TARGET})
		set(GEODE_NATIVE_CPP_TARGET "apache.geode::cpp")

GEODE-4343: Add auto-serialization example for .net #227

GEODE-4343: Add auto-serialization example for .net #227

Conversation

pdxcodemonkey commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jake-at-work left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jake-at-work commented Feb 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment