Skip to content

Conversation

jaredsburrows
Copy link

@@ -16,3 +17,7 @@ allprojects {
}
}
}

task wrapper(type: Wrapper) {
gradleVersion = '2.2.1'
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole task is not needed. Once the wrapper has been checked in you can just edit the properties file to update it.

@jaredsburrows
Copy link
Author

@JakeWharton I have removed the extra task and used the release build type.

@JakeWharton
Copy link
Contributor

Multidex works fine with Dagger 2. It just doesn't work with the current architecture. It would be trivial to alter the behavior.

@jaredsburrows
Copy link
Author

@JakeWharton You have used MultiDex + DI before? I have had issues with Roboguice + MultiDex in the past. What do you mean current architecture?

@JakeWharton
Copy link
Contributor

The documentation for multidex states that you need to defer having classes
loaded until the multidex classloader has been initialized. Otherwise the
class will be in the wrong classloader and won't be available.

The current design forces Dagger classes to be loaded before the multidex
classloader is initialized. This is the cause for the problems you are
seeing.

However, I don't think we should attempt to showcase absolutely every
feature of the Android build system in a single sample. Granular, explicit
samples end up being much more effective in my experience.
On Nov 27, 2014 9:06 PM, "Jared Burrows" [email protected] wrote:

@JakeWharton https://github.com/JakeWharton You have used MultiDex + DI
before? I have had issues with Roboguice + MultiDex in the past. What do
you mean current architecture?


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

@jaredsburrows
Copy link
Author

@JakeWharton I understand. I just wanted to be able to add Proguard to the working Android sample that uses Dagger 2.0. Should I remove the MultiDex line?

@JakeWharton
Copy link
Contributor

Yeah let's remove it. We'll make another app sample somewhere to showcase
multidex properly.
On Nov 27, 2014 9:15 PM, "Jared Burrows" [email protected] wrote:

@JakeWharton https://github.com/JakeWharton I understand. I just wanted
to be able to add Proguard to the working Android sample that uses Dagger
2.0. Should I remove the MultiDex line?


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

@jaredsburrows jaredsburrows changed the title Updates for build.gradle and tested proguard/multidex Updates for build.gradle and tested Proguard Nov 28, 2014
Added and tested Proguard
@jaredsburrows
Copy link
Author

  • Removed multidex
  • Updated commit message
  • Update pull request message

@JakeWharton Will you showcase Dagger 2.0 + MutliDex in a separate project soon? I was hoping to utilize both Proguard and MultiDex with Dagger 2.0.

@JakeWharton
Copy link
Contributor

Proguard should work without any actual changes. But yeah we can make a
multidex sample in the next week or so. We really should collect Dagger and
all these samples into a GitHub org so they're in one place...
On Nov 27, 2014 9:30 PM, "Jared Burrows" [email protected] wrote:

  • Removed multidex
  • Updated commit message
  • Update pull request message

@JakeWharton https://github.com/JakeWharton Will you showcase Dagger
2.0 + MutliDex in a separate project soon? I was hoping to utilize both
Proguard and MultiDex with Dagger 2.0.


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

@jaredsburrows
Copy link
Author

@JakeWharton Yes! I saw you posted that somewhere. I agree and I would like to contribute.

You posted here:
https://groups.google.com/forum/#!topic/dagger-discuss/q0AkspZ385c

@jaredsburrows
Copy link
Author

@gk5885 @JakeWharton Was there anything else that needed to change for this pull request?

@jaredsburrows
Copy link
Author

@gk5885 Any updates on this?

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.

2 participants