Skip to content
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

Changed GatewayObserverApi interface to have default methods #877

Merged

Conversation

zahariaca
Copy link
Contributor

Methods in GatewayObserverApi now are default.

  • GatewayObserverFactorySpi & GatewayObserverFactorySpiPrototype are not needed anymore and have been removed.
  • ManagementGatewayObserver has been changed to implement the interface.
  • GatewayObserver has been changed to use the interface instead of GatewayObserverFactorySpi to better adhere to the directive "program to an interface".

…rApi instead of extending GatewayObserverFactorySpiPrototype.java which doesn't exist anymore
Copy link
Contributor

@Anisotrop Anisotrop left a comment

Choose a reason for hiding this comment

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

GatewayObserverFactorySpi should be restored and used as an interface not as an abstract class. Also GatewayObserver should not be used as a declared type in constructor signatures and be replaced with GatewayObserverApi. Should also consider logging when injecting a resource fails in the loaded Spi.

@@ -150,7 +150,7 @@ public void destroyedService(ServiceContext serviceContext) {

@Override
public void startingGateway(GatewayContext gatewayContext) {
for (GatewayObserverFactorySpi gatewayListenerSpi : gatewayListenerSpi) {
for (GatewayObserverApi gatewayListenerSpi : gatewayListenerSpi) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should rename the first gatewayListenerSpi into gatewayListener.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@Anisotrop
Copy link
Contributor

Anisotrop commented Apr 6, 2017

The failed build is caused by: #822

@Anisotrop Anisotrop merged commit eb22f37 into kaazing:develop Apr 6, 2017
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