Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/409#discussion_r22144184
--- Diff: core/src/main/java/brooklyn/management/internal/UsageManager.java
---
@@ -56,11 +56,11 @@
public interface UsageListener {
public static final UsageListener NOOP = new UsageListener() {
@Override public void onApplicationEvent(String applicationId,
String applicationName, String entityType,
- Map<String, String> metadata, ApplicationEvent event)
{}
+ String catalogItemId, Map<String, String> metadata,
ApplicationEvent event) {}
@Override public void onLocationEvent(String locationId,
Map<String, String> metadata, LocationEvent event) {}
};
- void onApplicationEvent(String applicationId, String
applicationName, String entityType,
+ void onApplicationEvent(String applicationId, String
applicationName, String entityType, String catalogItemId,
--- End diff --
For a future PR (not here)... We should do something about these method
signatures. They are not good for future-proofing our API. If anyone implements
the method, then we'll break their code if we try to add additional parameters
in the future. We should group the parameters into a single param (e.g. an
interface with getters for each of those, saying in the interface's javadoc
that users are strongly discouraged from implementing it and that additional
properties may be added in future releases).
Maybe the right signature is `onApplicationEvent(String applicationId,
ApplicationMetadata metadata, ApplicationEvent event)`. The reason being:
applicationId is the uid of the app; the metadata tells us details of the app;
the event describes the state-change that is happening for the app.
The same argument goes for `onLocationEvent`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
