Skip to content

Conversation

@spmallette
Copy link
Contributor

https://issues.apache.org/jira/browse/TINKERPOP-1552

Presenting the Gremlin.Net GLV. It follows patterns for GLV generation established by gremlin-python and has build/deployment automation in maven. It does not have an extensive test suite given the JVM-centric nature of gremlin-test, but it does contain both unit and integration tests that should provide some confidence.

Upgrade documentation has not been updated yet since it isn't clear if we will officially release this with 3.2.6 or not. We may just do SNAPSHOT style releases until we gather more feedback. I suspect that we will have more changes to Gremlin.Net before 3.2.6 is ready for release, but the work seems stable enough to be in a release branch at this point.

Builds with docker nicely - @dkuppitz could you please take a look at the docker changes for this? not sure if anything else needs to be done (or done differently).

VOTE +1

FlorianHockmann and others added 26 commits June 28, 2017 15:13
This adds Gremlin-CSharp (a C# GLV), together with a .NET driver. The
driver is based on Gremlin.Net
(https://github.com/FlorianHockmann/Gremlin.Net) with added support for
GLVs and Gremlin-CSharp is auto generated, very similar to
Gremlin-Python.
This should fix TINKERPOP-1552.
This merges the formerly three .NET projects into just one: Gremlin.Net. For more information see the discussion in TINKERPOP-1552.
Removed a bunch of uncessary declarations. Build still seems to work nicely after all the deleting and moving stuff around.
Fixed a warning message in the build.
This mainly removes explicit type casting that are now obsolete thanks to changes from the pull request #620. It also makes the Dictionary CSharpToJavaEnums in NamingConversions private to avoid direct access to it as GetEnumJavaName should be used instead.
Moved GLV generation to the gremlin-dotnet pom and gmaveplus plugin.
This removes some obsolete configuration options and improves the package meta information. Especially the description was extended to reflect the current state of Gremlin-DotNet. This explanation can be removed as soon as the old Gremlin.Net driver is obsolete (probably when a first release version of Gremlin-DotNet is released).
The version is now 3.2.5-beta1.
Every build now generates an XML document containing the documentation comments which will be displayed to the user with IntelliSense. The warning about missing comments had to be disabled for some files as we currently just don't have comments for those.
Can't use the dotnet maven plugin on linux. it makes direct calls to nuget, which on linux requires mono. Used antrun plugin instead which is what was used with gremlin-python and works fine. Accomplished a basic push to the nuget staging environment. Still some bumps to sort out before this is good.
It's a bit hokey, but it works. There are some problems in the toolchain that makes this less nice that the gremlin-python setup. I almost wonder if we shouldn't copy the whole gremlin-dot-net source to target and operate on it in isolation so that we dont' have to muck with the source controlled csproj file. I suppose what I have here will work for now. As long as the documentation I wrote is followed I don't see any problems popping up, but we'll see.
@dkuppitz
Copy link
Contributor

I guess dotnet-dev-1.0.1 could be bumped to dotnet-dev-1.0.4 (that's currently the latest).

What is apt-transport-http needed for?

@FlorianHockmann
Copy link
Member

Yes, bumping dotnet dev to 1.0.4 makes sense. 1.0.1 was probably the newest version when I worked on the Dockerfile.

What is apt-transport-http needed for?

The .NET Core APT repos seem to only work with HTTPS. So apt-transport-https needs to be installed first.

@jorgebay
Copy link
Contributor

jorgebay commented Jul 3, 2017

Some minor nits:

  • Entries in root .gitignore file can be moved to gremlin-dotnet/.gitignore to avoid polluting the root file.
  • The coding style for generated enums is not very common, one declaration per line is expected in C#.

Apart from that, it looks good to me.

@spmallette
Copy link
Contributor Author

Made some adjustments:

  1. Bumped to dotnet-dev-1.0.4
  2. Fixed the enums so they are on separate lines

I left .gitignore as gremlin-python also pollutes that space atm and I wasn't too worried about it. I guess we can tweak that up whenever if it's bugging anyone. Any votes from committers now?

@jorgebay
Copy link
Contributor

I'm going to use my newly obtained committer status to say: +1!

@FlorianHockmann
Copy link
Member

Since most of the changes in this pull request are my own and I like the work in it from @spmallette and @jorgebay, I'll also use my new committer status to VOTE +1 on this one.

@spmallette
Copy link
Contributor Author

I will get this merged in the next day or so.

@spmallette
Copy link
Contributor Author

sadly the merge to master didn't go cleanly - see the new PR at #670 to get this to master. there are failing tests. @FlorianHockmann could you have a look at those and see if it is easy to correct? if so, feel free to just push your changes directly to that branch - much easier to do things now that you are a committer :) thanks.

@asfgit asfgit merged commit 426d141 into tp32 Jul 18, 2017
@asfgit asfgit deleted the TINKERPOP-1552 branch September 5, 2017 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants