Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract JanusGraph Gremlin driver requirements #1521

Closed

Conversation

farodin91
Copy link
Contributor

@farodin91 farodin91 commented Apr 3, 2019

Support things

  • Predicates
  • Geoshape
  • RelationIdenitifier

ToDo:

  • GraphSON against a janusgraph instance
    • geoshape
    • vertices
    • relationIdentifier
    • predicates
  • Gyro against a janusgraph instance
    • geoshape
    • vertices
    • relationIdentifier
    • predicates

Signed-off-by: Jan Jansen jan.jansen@gdata.de

Discussion on google groups: https://groups.google.com/forum/#!topic/janusgraph-dev/nmdEk_uGxdI


Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there an issue associated with this PR? Is it referenced in the commit message?
  • Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
  • Has your PR been rebased against the latest commit within the target branch (typically master)?
  • Is your initial contribution a single, squashed commit?

For code changes:

  • Have you written and/or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
  • If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?
  • If this PR is a documentation-only change, have you added a [skip ci]
    tag to the first line of your commit message to avoid spending CPU cycles in
    Travis CI when no code, tests, or build configuration are modified?

Note:

Please ensure that once the PR is submitted, you check Travis CI for build issues and submit an update to your PR as soon as possible.

Any thoughts?

@janusgraph-bot janusgraph-bot added the cla: yes This PR is compliant with the CLA label Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@JanusGraph JanusGraph deleted a comment Apr 3, 2019
@farodin91 farodin91 requested a review from porunov April 16, 2019 19:11
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@JanusGraph JanusGraph deleted a comment Apr 16, 2019
@porunov
Copy link
Member

porunov commented Apr 16, 2019

@farodin91 I will try to review it this week

@farodin91 farodin91 added this to the v0.5.0 milestone Jul 2, 2019
@JanusGraph JanusGraph deleted a comment Jul 23, 2019
@JanusGraph JanusGraph deleted a comment Jul 23, 2019
@JanusGraph JanusGraph deleted a comment Jul 23, 2019
@JanusGraph JanusGraph deleted a comment Jul 23, 2019
@JanusGraph JanusGraph deleted a comment Jul 23, 2019
@JanusGraph JanusGraph deleted a comment Jul 23, 2019
@farodin91 farodin91 added the cla: no This PR is not compliant with the CLA label Aug 21, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@JanusGraph JanusGraph deleted a comment Oct 7, 2019
@farodin91
Copy link
Contributor Author

@farodin91 I didn't review this PR yet but I see that Travis CI fails and Codacy has a lot of issues. I think that should be fixed.

I don't want to fix all codacy issue to prevent any major changes.

@porunov
Copy link
Member

porunov commented Oct 8, 2019

I don't want to fix all codacy issue to prevent any major changes

That is OK but I think at least CI build should pass

@farodin91 farodin91 force-pushed the extra-java-gremlin-driver branch 2 times, most recently from 17e4466 to 5c631cd Compare October 8, 2019 07:34
@JanusGraph JanusGraph deleted a comment Oct 8, 2019
@farodin91
Copy link
Contributor Author

@porunov I did break something by rebasing.

@JanusGraph JanusGraph deleted a comment Oct 14, 2019
@JanusGraph JanusGraph deleted a comment Dec 9, 2019
@farodin91 farodin91 force-pushed the extra-java-gremlin-driver branch 3 times, most recently from 3ce0343 to 52f0d7f Compare December 10, 2019 07:28
* Predicates
* Geoshape
* RelationIdenitifier

Signed-off-by: Jan Jansen <jan.jansen@gdata.de>
Copy link
Member

@porunov porunov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally I have reviewed all the code in this PR. All code LGTM. Thank you for moving it to janusgraph-driver, also thank you for some additional refactoring.
Even so it looks like everything should work, I would like to execute TinkerPop tests for all big PRs.
I've started TP tests for current master branch with merged this PR into it. I will post a result in about 15 hours.

Copy link
Member

@porunov porunov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you @farodin91 .
All TP tests passed.

@farodin91 farodin91 closed this Jan 3, 2020
@farodin91
Copy link
Contributor Author

@porunov I merged it locally using fast forward

@farodin91
Copy link
Contributor Author

b96aa6d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This PR is compliant with the CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants