-
Notifications
You must be signed in to change notification settings - Fork 656
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
Refactor package structure #368
Refactor package structure #368
Conversation
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.
A lot cleaner, thanks!
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.
Looks neat.
Since those are breaking changes, I think we should be doing an 0.2.2 release first then merge that after to be part of 0.3.0 snapshot?
gradle.properties
Outdated
@@ -1,13 +1,13 @@ | |||
GROUP=com.apollographql.android |
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.
We're good to change that to com.apollographql
according to https://issues.sonatype.org/browse/OSSRH-27585
@@ -25,6 +25,7 @@ class ApolloPlugin implements Plugin<Project> { | |||
private static final String NODE_VERSION = "6.7.0" | |||
public static final String TASK_GROUP = "apollo" | |||
private static final String APOLLO_DEP_GROUP = "com.apollographql.android" | |||
//TODO change this to apollo-runtime when it will be published |
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.
issue pls
I think the better approach would be to comment out the line that adds the runtime dependency and disable the plugin tests.
It's better to not package that artifact at all instead of packaging the wrong one.
apollo-rxsupport/gradle.properties
Outdated
@@ -1,4 +1,4 @@ | |||
POM_ARTIFACT_ID=RxSupport | |||
POM_ARTIFACT_ID=apollo-rx-support | |||
POM_NAME=ApolloStack RxSupport |
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.
unrelated but Apollo GraphQL RxSupport
?
Same for other gradle.properties
@@ -5,8 +5,4 @@ | |||
/** TODO **/ | |||
public interface ResponseReader { | |||
<T> T read(Field field) throws IOException; | |||
|
|||
interface ValueHandler { |
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.
what happened to interface?
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.
It's not used, legacy (forgot to remove when we refactored response readers)
Lgtm |
@@ -14,7 +14,7 @@ jdk: | |||
- oraclejdk8 | |||
|
|||
script: | |||
- ./gradlew clean build connectedCheck -x checkstyleTest --stacktrace | |||
- ./gradlew clean build connectedCheck -x checkstyleTest -x :apollo-integration:compileDebugUnitTestJavaWithJavac -x :apollo-integration:compileReleaseUnitTestJavaWithJavac --stacktrace |
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.
Can do -Dapollographql.skipRuntimeDep=true -x apollo-gradle-plugin:test
faster than commenting out next time 😀
Closes #250
New package structure: