SteelBridgeLabs / neo4j-gremlin-bolt

Apache License 2.0
0 stars 1 forks source link

added tests for more types in a uniform manner #80 #82

Open phreed opened 5 years ago

phreed commented 5 years ago

This change:

rjbaucells commented 5 years ago

Please do the pull request against the development branch. Do not change the release version in the pom.xml, I will do it at the time of preparing the next release. Make sure the code is properly formatted, I see it is not following the current formatting we have in the code.

phreed commented 5 years ago

What type of formatting difference are you referring to? Do you mean replacing x || y || z || return with if (x) return ?

phreed commented 5 years ago

Also, I am overloading existing tests with additional properties by way of the

@Rule
    public ErrorCollector collector = new ErrorCollector();

Is this acceptable or should I be looking at JUnit5 and Dynamic tests?

rjbaucells commented 5 years ago

Check code formatting against existing code and you will see the differences, as a quick example I saw the for statement with extra spaces within parenthesis. Also test are failing, see build in Travis. The PR should be against the development branch, see previous comments...

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.4%) to 57.785% when pulling 36bc3e5c5f54c60b63e3fd2ec0f792644b60b642 on babeloff:master into df3ab429e0c83affae6cd43d41edd90de80c032e on SteelBridgeLabs:development.

phreed commented 5 years ago

It seems to be working both in the tests and for my application making use of the feature. What more do I need to do to clean this up?

There should probably be some documentation about how to create Point properties that work. Here is a sample...

val location = DriverValues.point(CoordinateReferenceSystem.WGS84.code, lat ?: 0.0, lon ?: 0.0).asPoint()
g.V().has("foo","bar","baz").property(key, location).iterate();
phreed commented 5 years ago

@rjbaucells What is this waiting on? Is there something more I need to do?

rjbaucells commented 5 years ago

@phreed See my comments on this PR...