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
Separate java and android builds #188
Conversation
This introduces a "java" subproject for the JRE-specific version of the lib, leaving the "lib" subproject containing the common code. The normal jar also contains the common lib classes from the lib subproject, without any other dependencies. There continues to be a fullJar task to generate a jar containing all dependencies. The Maven upload is the former, normal, jar, with dependencies set appropriately. The Maven upload no longer contains javadoc or source classes. If it did, the jars would be pretty much empty, and I did not spend much time trying to figure out how to get the javadoc or source from the common code into the jars here.
The previous commit renamed the "android-test" subproject to "android". This commit modifies it to generate the Android-specific version of the lib, and run tests on it. The resulting AAR also contains the common lib classes from the lib subproject. The Maven upload has dependencies set appropriately. The Maven upload does not contain javadoc or source classes. If it did, the jars would be pretty much empty, and I did not spend much time trying to figure out how to get the javadoc or source from the common code into the jars here. There are some extra test failures over and above what you get on the JRE-specific library. I have not investigated these yet.
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.
Pls have a look at the comments but looks good.
|
||
gradle android:assemble | ||
|
||
(The `ANDROID_HOME` environment variable must be set appropriately.) |
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 have a requirement that if ANROID_HOME
isn't set, and the Android SDK isn't installed, then it will still build and run the java subproject. Can you check that that still works pls?
* Run `gradle java:assemble` to build the JRE-specific JARs for this release | ||
* Run `gradle android:assemble` to build the Android AAR for this release | ||
* Visit [https://github.com/ably/ably-java/tags](https://github.com/ably/ably-java/tags) and `Add release notes` for the release, then attach the generated JARs (`ably-java-0.8.4.jar` and `ably-java-0.8.4-full.jar`) in the folder `java/build/libs`, | ||
and the generated AAR (`ably-android-0.8.4-release.aar` in the folder `android/build/outputs/aar`. | ||
|
||
### Publishing to JCentre (Maven) |
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.
JCenter I think
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 didn't write that, but I can add a commit to fix it.
// in java/build.gradle and android/build.gradle for maven. | ||
dependencies { | ||
compile 'org.msgpack:msgpack-core:0.8.2' | ||
compile 'org.java-websocket:Java-WebSocket:1.3.1' |
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.
Aren't we using the forked version of this? I thought we had done a rename of the package as well as the groupId.
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 didn't change this, I just moved it from lib/build.gradle to dependencies.gradle.
@@ -10,7 +10,7 @@ | |||
public byte[] read(String resourceName) throws IOException { | |||
FileInputStream fis = null; | |||
try { | |||
fis = new FileInputStream(new File("src/test/resources", resourceName)); | |||
fis = new FileInputStream(new File("../lib/src/test/resources", resourceName)); |
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.
Is there a requirement to be in a particular directory in order to run the tests?
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.
Yes, looks like it will only work when run from the root of your ably-java checkout.
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.
Er, I mean no, that path must be relative to the subproject, even though I can run the tests in the root of the ably-java checkout. It seems to work.
@@ -53,6 +68,12 @@ Download [the latest JAR](https://github.com/ably/ably-java/releases) or grab vi | |||
compile 'io.ably:ably-java:0.8.4' |
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.
Wouldn't this be 0.8.5?
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.
Yes, but there are several references to 0.8.4 in README.md, and the "release notes" instruction says to update them when you are doing a new version. So it will get changed when someone does a release.
Travis build does not look any worse than normal. Although it is only testing JRE; we don't have anything to test android yet. |
Thanks |
Supersedes PR #128