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

Re organize sample #82

Closed
wants to merge 3 commits into from
Closed

Conversation

zsiegel
Copy link

@zsiegel zsiegel commented Nov 25, 2014

Starting work on #59

import android.content.Context;
import android.content.Intent;
import rx.Observable;
import rx.android.samples.lifeycle.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use star imports.

@zsiegel
Copy link
Author

zsiegel commented Nov 25, 2014

do you want me to squash again?

@JakeWharton
Copy link
Contributor

Yes, please! I wish GitHub had a squash button on PRs.

@zsiegel
Copy link
Author

zsiegel commented Nov 25, 2014

rebased.

@mttkay
Copy link
Collaborator

mttkay commented Nov 26, 2014

Needs another rebase... this project is moving too quickly :-)

@zsiegel
Copy link
Author

zsiegel commented Nov 26, 2014

@mttkay ok so I did a rebase and pulled in the changes from 0.x but for some odd reason its still saying merge conflict.. So I think that is my issue however it looks like the changes to the build.gradle file are causing issues running the samples app.

Gradle spits out a null pointer exception with no error message.

@zsiegel
Copy link
Author

zsiegel commented Nov 26, 2014

@mttkay fixed the rebase issues. i still cant get it to build in intellij anymore. tried re-importing and using different versions on intellij.

Can you let me know if it launches ok for you. I dont want to say this is good to go until i can launch the sample app.

@zsiegel
Copy link
Author

zsiegel commented Nov 26, 2014

> startup failed:
  Could not instantiate global transform class org.spockframework.compiler.SpockTransform specified at jar:file:/Users/zsiegel/.gradle/caches/modules-2/files-2.1/org.spockframework/spock-core/0.7-groovy-1.8/3a677d19e8d3acf3bd296c4023356256d55da5a3/spock-core-0.7-groovy-1.8.jar!/META-INF/services/org.codehaus.groovy.transform.ASTTransformation  because of exception org.spockframework.util.IncompatibleGroovyVersionException: The Spock compiler plugin cannot execute because Spock 0.7.0-groovy-1.8 is not compatible with Groovy 2.3.6. For more information, see http://versioninfo.spockframework.org
  Spock location: file:/Users/zsiegel/.gradle/caches/modules-2/files-2.1/org.spockframework/spock-core/0.7-groovy-1.8/3a677d19e8d3acf3bd296c4023356256d55da5a3/spock-core-0.7-groovy-1.8.jar
  Groovy location: file:/Users/zsiegel/.gradle/wrapper/dists/gradle-2.1-all/27drb4udbjf4k88eh2ffdc0n55/gradle-2.1/lib/groovy-all-2.3.6.jar

  1 error

@JakeWharton
Copy link
Contributor

Are you sure you're using the gradle wrapper to build? It's set a 1.12 not 2.1.

You can see #81 for the code needed to workaround this.

@zsiegel
Copy link
Author

zsiegel commented Nov 26, 2014

Yes I've been trying to pay close attention to that knowing we are pretty behind and I saw your attempt at bringing that up to the latest.

I can run things at the command line using the wrapper without issue. I have installed the sample app from the command line and things are OK so this is fine to merge IMO.

BUT I think intelliJ is having issues with the current state of 0.x.

For example I cloned 0.x into a new directory, imported into Intellij, and tried to run the sample and it errors out. I have been running this project in 14.0.1 because obviously newer EAP versions require updated Android tools versions.

Just want to make sure we didn't make a change that prevents people from using this fully in an IDE. I added the debug flags to my intellij and tried to run the project so we can see whats going on here. Screenshot is attached. If this will be addressed soon with your updated PR please just ignore this for now. Have I missed something in configuring the project?

screen shot 2014-11-26 at 12 10 26 pm

Thanks all!

@JakeWharton
Copy link
Contributor

I also experienced that problem, but I cannot recall the fix. I haven't yet opened this project in an IDE. Been doing all my changes from Sublime.

@JakeWharton
Copy link
Contributor

I also don't have such an old version of the build tools installed so I've been letting Travis CI do the compilation for me 😀

@zsiegel
Copy link
Author

zsiegel commented Nov 26, 2014

Alright no problem. Looking forward to getting the build tools to the latest anyways. Ill manage for now.

This is fine on my end.

@dlew
Copy link
Collaborator

dlew commented Nov 26, 2014

FWIW, I can no longer get IntelliJ to build anymore either. Command-line continues to work.

@zsiegel
Copy link
Author

zsiegel commented Nov 26, 2014

:( yeah I think we may have messed something up with the changes to gradle file. hopefully we can get this updated to the newer tools versions to help.

@mttkay there are a few PR's with updates to the sample app. can we get this merged in before those?

thanks!

@mttkay
Copy link
Collaborator

mttkay commented Nov 27, 2014

I'm on leave until Monday and will try to stay clear of any computers for a
change, but @JakeWharton can merge it. It looked good to me.
On Nov 26, 2014 10:57 PM, "Zac Siegel" [email protected] wrote:

:( yeah I think we may have messed something up with the changed to gradle
file. hopefully we can get this updated to the newer tools versions to help.

@mttkay https://github.com/mttkay there are a few PR's with updates to
the sample app. can we get this merged in before those?

thanks!


Reply to this email directly or view it on GitHub
#82 (comment).

@mttkay
Copy link
Collaborator

mttkay commented Dec 3, 2014

Sorry about the hold up. I would like this to land first: #81

Then this needs a rebase (again)

@zsiegel
Copy link
Author

zsiegel commented Dec 3, 2014

No problem I'll get it rebased.

Zac Siegel
On Dec 3, 2014 4:11 AM, "Matthias Käppler" [email protected] wrote:

Sorry about the hold up. I would like this to land first: #81
#81

Then this needs a rebase (again)


Reply to this email directly or view it on GitHub
#82 (comment).

@JakeWharton
Copy link
Contributor

As part of #172 the sample has been gutted and no longer needs reorganizing. Thanks for your efforts though.

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

Successfully merging this pull request may close these issues.

4 participants