Skip to content

Upgrade Dagger dependency from 1.1.0 to 1.2.2 #124

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.

Sign up for GitHub

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

Jump to bottom
Merged
codefromthecrypt merged 2 commits into from
Dec 19, 2014
Merged

Upgrade Dagger dependency from 1.1.0 to 1.2.2 #124

codefromthecrypt merged 2 commits into from
Dec 19, 2014

Conversation

@qualidafial
Copy link
Contributor

@qualidafial qualidafial commented Nov 4, 2014

Feign's dependency on Dagger is nearly a year out of date.

In version 1.2.0, Dagger introduced a backward-incompatible change to dagger.internal.ModuleAdapter, by adding a new argument Class<?> moduleClass to the constructor.

This incompatibility causes Feign.builder() to fail with a NoSuchMethodError, when using newer versions of Dagger:

java.lang.NoSuchMethodError: dagger.internal.ModuleAdapter.<init>([Ljava/lang/String;[Ljava/lang/Class;Z[Ljava/lang/Class;ZZ)V
    at feign.Feign$Defaults$$ModuleAdapter.<init>(Feign$Defaults$$ModuleAdapter.java:22)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:408)
    at java.lang.Class.newInstance(Class.java:438)
    at dagger.internal.Loader.instantiate(Loader.java:84)
    at dagger.internal.FailoverLoader$1.create(FailoverLoader.java:42)
    at dagger.internal.FailoverLoader$1.create(FailoverLoader.java:40)
    at dagger.internal.Memoizer.get(Memoizer.java:56)
    at dagger.internal.FailoverLoader.getModuleAdapter(FailoverLoader.java:57)
    at dagger.internal.Modules.loadModules(Modules.java:43)
    at dagger.ObjectGraph$DaggerObjectGraph.makeGraph(ObjectGraph.java:174)
    at dagger.ObjectGraph$DaggerObjectGraph.access$000(ObjectGraph.java:138)
    at dagger.ObjectGraph.create(ObjectGraph.java:129)
    at feign.Feign$Builder.<init>(Feign.java:182)
    at feign.Feign.builder(Feign.java:55)

This PR upgrades Dagger to the latest release. I've also added a warning to CHANGES.md regarding Dagger generated code compatibility.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Nov 4, 2014

Thanks tons! @qualidafial

@quidryan @allenxwang do you know if dagger 1.2.2 is blessed, yet? I want to make sure before I merge this.

@cloudbees-pull-request-builder
Copy link

cloudbees-pull-request-builder commented Nov 5, 2014

feign-pull-requests #180 SUCCESS
This pull request looks good

@qualidafial
Copy link
Contributor Author

qualidafial commented Nov 7, 2014

ping

@qualidafial
Copy link
Contributor Author

qualidafial commented Nov 14, 2014

Ping. Is there anything I can do to help push this through?

@qualidafial
Copy link
Contributor Author

qualidafial commented Dec 3, 2014

Ping?

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Dec 3, 2014

@qualidafial sorry I can't merge this until someone from netflix confirms that the dependency is whitelisted internally.

@tbak
Copy link

tbak commented Dec 17, 2014

@qualidafial @adriancole I have added dagger 1.2.2 to netflix artifactory.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Dec 17, 2014

Thanks. There has been persistent concern about being able to do a release.
If we get build green etc, could you or someone else help cut one from a
branch we prepare?

@tbak
Copy link

tbak commented Dec 18, 2014

Sure, I can help with making the release.

On Wed, Dec 17, 2014 at 9:20 AM, Adrian Cole notifications@github.com
wrote:

Thanks. There has been persistent concern about being able to do a release.
If we get build green etc, could you or someone else help cut one from a
branch we prepare?

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

codefromthecrypt pushed a commit that referenced this pull request Dec 19, 2014
@adriancole
Merge pull request #124 from qualidafial/master
43af8fe 
Upgrade Dagger dependency from 1.1.0 to 1.2.2
@codefromthecrypt codefromthecrypt merged commit 43af8fe into OpenFeign:master Dec 19, 2014
@quidryan
Copy link
Contributor

quidryan commented Dec 19, 2014

@adriancole: @rspieldenner is working a new Gradle plugin to take care of releases. It will follow the ReactiveX model, and should releasing back into your hands. I'll leave it up to him to give you updates.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Dec 19, 2014

oh that would be sooo helpful! cc @brianm

On Fri, Dec 19, 2014 at 10:21 AM, Justin Ryan notifications@github.com
wrote:

@adriancole https://github.com/adriancole: @rspieldenner
https://github.com/rspieldenner is working a new Gradle plugin to take
care of releases. It will follow the ReactiveX model, and should releasing
back into your hands. I'll leave it up to him to give you updates.

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

@rspieldenner
Copy link
Contributor

rspieldenner commented Dec 19, 2014

I'm testing out https://github.com/nebula-plugins/gradle-netflixoss-project-plugin with https://github.com/Netflix/netlixoss-build-infrastructure today. Will send some pull requests to you when I'm ready.

velo pushed a commit that referenced this pull request Oct 8, 2024
@adriancole
Merge pull request #124 from qualidafial/master
eb86e78 
Upgrade Dagger dependency from 1.1.0 to 1.2.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@qualidafial @codefromthecrypt @cloudbees-pull-request-builder @tbak @quidryan @rspieldenner