WELD-2359 Introduce WeldNotificationsOptions #59
Conversation
| * @author Martin Kouba | ||
| * @see Event#fireAsync(Object, NotificationOptions) | ||
| */ | ||
| public final class AsyncNotifications { |
There was a problem hiding this comment.
Hm I don't think it's an appropriate name. What about AsyncNotificationOptions then? Or AsyncEventOptions.
There was a problem hiding this comment.
-1 for AsyncNotificationOptions which would indicate a subinterface/impl of NotificationOptions.
I'm not sure about AsyncEventOptions...
There was a problem hiding this comment.
I also think AsyncNotificationOptions is better.
| * | ||
| * @see NotificationMode | ||
| */ | ||
| public static final String MODE = "org.jboss.weld.async.notification.mode"; |
There was a problem hiding this comment.
I would go only with weld.async.notification.mode
| * @author Martin Kouba | ||
| * @see Event#fireAsync(Object, NotificationOptions) | ||
| */ | ||
| public final class AsyncNotifications { |
There was a problem hiding this comment.
I also think AsyncNotificationOptions is better.
|
|
||
| /** | ||
| * | ||
| * @param timeout |
There was a problem hiding this comment.
We should note that this is in milliseconds.
| public static final String MODE = "org.jboss.weld.async.notification.mode"; | ||
|
|
||
| /** | ||
| * Makes it possible to specify a timeout after which the returned completion stage must be complete. |
There was a problem hiding this comment.
Maybe we should elaborate on the possible cases? E.g. when you get exception and what exception because you might be handling multiple exceptions in you exceptionally block.
| /** | ||
| * Makes it possible to specify a timeout after which the returned completion stage must be complete. | ||
| * <p> | ||
| * Timeout expiration does not abort the notification of the observers. |
There was a problem hiding this comment.
Minor thing, I would rather say Time expiration
mkouba
changed the title
To discuss:
I decided not to introduce aNotificationOptionssubinterface but a final class with constants instead - I can see no value of having a Weld-specific implall option keys haveorg.jboss.weld.async.notificationprefix, maybe it's too long, I just wanted to have it clearorg.jboss.weld.eventspackage name -> due to JDK9 package clash