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

[2.6.x]: Added Java APIs for general dependency-injection to be DI framework agnostic #8586

Merged
merged 1 commit into from Sep 18, 2018

Conversation

Projects
None yet
2 participants
@FranklinYinanDing
Contributor

FranklinYinanDing commented Aug 28, 2018

…gnostic

Backport #8559 to 2.6.x branch.

@marcospereira marcospereira changed the title from Added Java APIs for general dependency-injection to be DI framework a… to [2.6.x]: Added Java APIs for general dependency-injection to be DI framework agnostic Aug 31, 2018

@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Sep 4, 2018

Hey @marcospereira is there anything else need to be addressed for checking in? Thanks!

@marcospereira

LGTM.

Thank you, @FranklinYinanDing.

I think as soon as we get a green build it will be okay to merge.

@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Sep 10, 2018

@marcospereira Thanks for reviewing!

I've corrected the real errors buried by other false positive ones, and verified locally.

But I noticed that the CI job wasn't kicked off by my commit. Is there something wrong with the CI build?

@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Sep 11, 2018

NVM! Maybe just some delay. All green now!

@marcospereira

Small comment and after that let's merge it. :-)

Thanks, @FranklinYinanDing.

import org.specs2.mutable.Specification
import play.{ Environment => EnvironmentJ }

This comment has been minimized.

@marcospereira

marcospereira Sep 17, 2018

Member

Nitpicks: noticed this while I was reviewing the other PR. Let's use Something => JavaSomething instead of Something => SomethingJ. :-)

This comment has been minimized.

@FranklinYinanDing

FranklinYinanDing Sep 17, 2018

Contributor

Done! That was our internal naming convention. I'll keep this in mind for future PR.

@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Sep 17, 2018

@marcospereira Could you please take a look at the latest CI failure? java.lang.NoClassDefFoundError: sbt/State$Next

I don't think my change causes this. Thank you!

@marcospereira

This comment has been minimized.

Member

marcospereira commented Sep 17, 2018

@FranklinYinanDing I've triggered the job again. It may be a problem with Travis cache.

@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Sep 17, 2018

Not sure if the latest one is the one you triggered, (the time looks like it), but it is still failing :(

@marcospereira

This comment has been minimized.

Member

marcospereira commented Sep 18, 2018

We got a green build after cleaning Travis cache. :-)

@marcospereira

👍

Thanks for your patience here, @FranklinYinanDing.

@marcospereira marcospereira merged commit 8ce38ef into playframework:2.6.x Sep 18, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details
@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Sep 18, 2018

Thank you for reviewing! I'll start working on the master branch one.

@marcospereira marcospereira added this to the Play 2.6.20 milestone Oct 9, 2018

@marcospereira

This comment has been minimized.

Member

marcospereira commented Oct 9, 2018

@FranklinYinanDing this is part of Play 2.6.20 released just a few hours ago. :-)

@FranklinYinanDing

This comment has been minimized.

Contributor

FranklinYinanDing commented Oct 9, 2018

Great! Thank you! I'm picking it up right now. @marcospereira

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment