-
Notifications
You must be signed in to change notification settings - Fork 59
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
PHOENIX-7366 Use Phoenix 5.2 to compile phoenix-connectors #133
Conversation
also refactor maven project setup also merge phoenix5-spark3-it back into phoenix5-spark module
The actal java changes are minimal, most of the changes is optimizing the pom.xmls, and merging the spark3 ITs back into the main module. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think It could have been better if phoenix 5.2 support was separated from the refactoring.
I can't comment it on the file but please remove the empty phoenix5-hive/README.md file at https://github.com/stoty/phoenix-connectors/blob/PHOENIX-7366/phoenix5-hive/README.md
otherwise It looks Good to me
phoenix5-spark/pom.xml
Outdated
<artifactId>maven-dependency-plugin</artifactId> | ||
<configuration> | ||
<ignoredUnusedDeclaredDependencies> | ||
<!-- These are test runtime dependencies that Maven has conecnt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
@@ -34,13 +34,16 @@ | |||
import org.junit.Ignore; | |||
import org.junit.Test; | |||
import org.junit.experimental.categories.Category; | |||
import org.slf4j.Logger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is beeing used
phoenix5-spark3/pom.xml
Outdated
<ignoredUsedUndeclaredDependencies> | ||
<!-- No trace of these in the source. Perhaps used anonymously | ||
in chained method calls ? --> | ||
<ignoredUsedUndeclaredDependency>joda-time:joda-time</ignoredUsedUndeclaredDependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,90 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not used anywhere, could you please remove it?
Thank you, @richardantal I have fixed the issues you found. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
merged manually |
also refactor maven project setup
also merge phoenix5-spark3-it back into phoenix5-spark module