-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47440/#review134811
-----------------------------------------------------------



LGTM. Only minor comments/questions left.


src/main/java/org/apache/aurora/scheduler/events/Webhook.java (line 47)
<https://reviews.apache.org/r/47440/#comment199720>

    This initialization can be inlined now.



src/main/java/org/apache/aurora/scheduler/events/Webhook.java (line 91)
<https://reviews.apache.org/r/47440/#comment199721>

    <p>



src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java (line 34)
<https://reviews.apache.org/r/47440/#comment199722>

    Don't you want to expose this all the way through to json? Fine if you 
don't but could be handy if you want to tune it one day.
    
    Also, we generally put 'static final' fields first.



src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java (line 65)
<https://reviews.apache.org/r/47440/#comment199725>

    Should be 4 spaces offset followed by empty line after '{'



src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java (line 67)
<https://reviews.apache.org/r/47440/#comment199728>

    You don't have to wrap with `requireNonNull` here as `copyOf` already 
throws on null.



src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java (line 75)
<https://reviews.apache.org/r/47440/#comment199729>

    I'd expect connection timeout to be here as well.


- Maxim Khutornenko


On May 25, 2016, 1:02 a.m., Dmitriy Shirchenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47440/
> -----------------------------------------------------------
> 
> (Updated May 25, 2016, 1:02 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Stephan Erb.
> 
> 
> Bugs: AURORA-1683
>     https://issues.apache.org/jira/browse/AURORA-1683
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Looking for some feedback whether I'm on the correct path in adding a 
> webhook. All comments are welcome!
> 
> 
> Diffs
> -----
> 
>   docs/reference/scheduler-configuration.md 
> f7d676d0ed6bc536f4341dbb9365cf50e8607efb 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 3d9e706de564df5e24cb34265bebc0db1cad11a0 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 9ebfe230836e88a97bc60092373f72f176a8f6f2 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 2a4c0665e48d30e0655de00bd7f6f9b49f01eafc 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> PRE-CREATION 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47440/diff/
> 
> 
> Testing
> -------
> 
> Need to fix tests.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Reply via email to