Skip to content

Upgrade Dagger dependency from 1.1.0 to 1.2.2#124

Merged
codefromthecrypt merged 2 commits into
OpenFeign:masterfrom
qualidafial:master
Dec 19, 2014
Merged

Upgrade Dagger dependency from 1.1.0 to 1.2.2#124
codefromthecrypt merged 2 commits into
OpenFeign:masterfrom
qualidafial:master

Conversation

@qualidafial

@qualidafial qualidafial commented Nov 4, 2014

Copy link
Copy Markdown
Contributor

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

codefromthecrypt commented Nov 4, 2014

Copy link
Copy Markdown

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

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

Copy link
Copy Markdown

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

@qualidafial

qualidafial commented Nov 7, 2014

Copy link
Copy Markdown
Contributor Author

ping

@qualidafial

qualidafial commented Nov 14, 2014

Copy link
Copy Markdown
Contributor Author

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

@qualidafial

qualidafial commented Dec 3, 2014

Copy link
Copy Markdown
Contributor Author

Ping?

@codefromthecrypt

codefromthecrypt commented Dec 3, 2014

Copy link
Copy Markdown

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

@tbak

tbak commented Dec 17, 2014

Copy link
Copy Markdown

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

@codefromthecrypt

codefromthecrypt commented Dec 17, 2014

Copy link
Copy Markdown

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

tbak commented Dec 18, 2014

Copy link
Copy Markdown

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

quidryan commented Dec 19, 2014

Copy link
Copy Markdown
Contributor

@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

codefromthecrypt commented Dec 19, 2014

Copy link
Copy Markdown

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

rspieldenner commented Dec 19, 2014

Copy link
Copy Markdown
Contributor

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