[GitHub] incubator-griffin pull request #444: Define griffin plain-vanilla hook.

2018-11-08 Thread gavlyukovskiy
Github user gavlyukovskiy commented on a diff in the pull request: https://github.com/apache/incubator-griffin/pull/444#discussion_r231981734 --- Diff: service/src/main/java/org/apache/griffin/core/event/GriffinEventListeners.java --- @@ -0,0 +1,50 @@ +package

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

2018-11-08 Thread guoyuepeng
Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Based on previous design discuss and code review. LGTM now. Any comments from you? @chemikadze ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

2018-11-08 Thread guoyuepeng
Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 I am ok for registration through declaration in properties. my problem is can we write triggered on called test case for in EventServiceTests. ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

2018-11-08 Thread gavlyukovskiy
Github user gavlyukovskiy commented on the issue: https://github.com/apache/incubator-griffin/pull/444 > Can we provide several individually implements in configuration like GriffinHook1, GriffionHook2, and autowired all individual hooks instances into List? You can define all

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

2018-11-08 Thread guoyuepeng
Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 >> > Can we provide several individually implements in configuration like GriffinHook1, GriffionHook2, and autowired all individual hooks instances into List? >> >> You can

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

2018-11-08 Thread gavlyukovskiy
Github user gavlyukovskiy commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Yes, I showed this in my example - take all hooks from the context, but use only those that configured in property. ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

2018-11-08 Thread gavlyukovskiy
Github user gavlyukovskiy commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Left a comment regarding injection. Tests didn't catch it because you explicitly specified all hooks that are registered in the context, I would add one more and leave it unused