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

[ZEPPELIN-3882] Neo4jInterpreter - Support Point and Date Types #3244

Merged
merged 3 commits into from
Dec 20, 2018

Conversation

conker84
Copy link
Contributor

What is this PR for?

Add the support for Point and Date data types introduced since Neo4j 3.4

What type of PR is it?

[Improvement] In order to allow users to use the Neo4j Interpreter with the last introduced Data types

Todos

  • - Bumped Neo4j dependecies version
  • - Added new data types to test

What is the Jira issue?

How should this be tested?

  • Execute the unit tests

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

@conker84 conker84 changed the title Update neo4j datatypes [ZEPPELIN-3882] Neo4jInterpreter - Support Point and Date Types Nov 30, 2018
@conker84
Copy link
Contributor Author

@felixcheung I'm back.

@felixcheung
Copy link
Member

looks like you are blocked by #3243

https://api.travis-ci.org/v3/job/461718800/log.txt

@conker84
Copy link
Contributor Author

conker84 commented Dec 2, 2018

Can you give me a ping when it's ready?

@zjffdu
Copy link
Contributor

zjffdu commented Dec 5, 2018

@conker84 #3243 is fixed, could you rebase this PR ?

@conker84
Copy link
Contributor Author

conker84 commented Dec 5, 2018

I did it, and travis it's green now:
https://travis-ci.org/conker84/zeppelin

@conker84
Copy link
Contributor Author

conker84 commented Dec 7, 2018

@zjffdu ping

@conker84
Copy link
Contributor Author

@zjffdu @felixcheung ping

@conker84
Copy link
Contributor Author

Still no news?

@felixcheung
Copy link
Member

I see it's green. not sure why the report above is not.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

LGTM

@felixcheung
Copy link
Member

merging if no more comment

@felixcheung felixcheung merged commit 5542262 into apache:master Dec 20, 2018
@felixcheung
Copy link
Member

it looks like the github rebase and merge option is picking the last commit description

@jongyoul
Copy link
Member

@felixcheung I think it would be better to squash and merge with some edit of title indicating an issue number. WDYT?

@felixcheung
Copy link
Member

felixcheung commented Dec 20, 2018 via email

@jongyoul
Copy link
Member

I've tested my wip pr by clicking squash and merge button on the pr page. I saw we could edit a title and commit messages as well. Could you please check it again?

@felixcheung
Copy link
Member

Squash and merge seems like the right option,
https://help.github.com/articles/merging-a-pull-request/

@felixcheung
Copy link
Member

@conker84 sorry, could you see if you could re open this PR so I can merge again?

@jongyoul
Copy link
Member

@felixcheung Thanks a lot 👍

@felixcheung
Copy link
Member

@conker84 ping - this was reverted, could you re-open

@conker84
Copy link
Contributor Author

@felixcheung sorry I lost this, do you mean open a new issue? Because I cannot reopen this one

@felixcheung
Copy link
Member

hm, this is a bit tricky - could you open a new branch and cherry-pick the commits to it, then open a new PR? @conker84

@conker84
Copy link
Contributor Author

@felixcheung I created the #3284. Let me know if I have to do something else

@felixcheung
Copy link
Member

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants