refactored the code to avoid creation two LifecycleInjector objects (and... #45
Conversation
…and two root Guice injectors)
|
karyon-pull-requests #14 FAILURE |
| @@ -75,7 +78,7 @@ | |||
| before returning from {@link com.google.inject.Module#configure(Binder)}.</li> | |||
| <li>{@link ServerBootstrap#configureBinder}: Callback to configure {@link Binder} before returning from | |||
| {@link BootstrapModule#configure(com.netflix.governator.guice.BootstrapBinder)}.</li> | |||
| <li>{@link ServerBootstrap#beforeInjectorCreation(com.netflix.governator.guice.LifecycleInjectorBuilder)}: A callback | |||
There was a problem hiding this comment.
Any specific reason why we are breaking backward compatibility here? beforeInjectorCreation() is the point where apps can add their modules to governator
|
I think it will be better to leave ServerBootstrap.beforeInjectorCreation() as is and provide the overridable method createInjector() as you already have. |
| @@ -170,7 +169,7 @@ public KaryonServer(@Nullable ServerBootstrap bootstrap) { | |||
| * {@link ServerBootstrap}. <br/> | |||
| * This method must be called during the server initialization. | |||
| * | |||
| * @return Guice's injector instance that will be used by <a href="https://github.com/Netflix/governator/">Governator</a> | |||
| * @return main injector instance that will be used by <a href="https://github.com/Netflix/governator/">Governator</a> | |||
There was a problem hiding this comment.
At the cost of this sounding nitpicking, I think it is confusing :)
I know the context w.r.t how we use it inside Netflix, but "Guice's injector" is much more clear as karyon only has a single injector.
|
karyon-pull-requests #18 SUCCESS |
|
karyon-pull-requests #19 SUCCESS |
refactored the code to avoid creation two LifecycleInjector objects (and...
|
karyon-pull-requests #20 SUCCESS |
... two root Guice injectors)