Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Joshua Cohen

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

Ship it!


lgtm


src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
https://reviews.apache.org/r/25593/#comment92730

Obviously not related, but should these all use Objects.requireNonNull 
instead of Preconditions.checkNotNull?


- Joshua Cohen


On Sept. 12, 2014, 9:28 p.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25593/
 ---
 
 (Updated Sept. 12, 2014, 9:28 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This paves the way for ShiroWebModule integration. As an added benefit Guice 
 is now instantiating our servlets for us (no injector.getInstance calls in 
 our code).
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 9d8654b4268a13fc9d29d12f8da2c1aa8fda4b2b 
 
 Diff: https://reviews.apache.org/r/25593/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 No new tests added - the existing JerseyServletModuleTests already cover 
 correctness of wiring.
 
 
 Thanks,
 
 Kevin Sweeney
 




Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney

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

(Updated Sept. 12, 2014, 2:39 p.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.


Changes
---

+jcohen


Repository: aurora


Description
---

This paves the way for ShiroWebModule integration. As an added benefit Guice is 
now instantiating our servlets for us (no injector.getInstance calls in our 
code).


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
9d8654b4268a13fc9d29d12f8da2c1aa8fda4b2b 

Diff: https://reviews.apache.org/r/25593/diff/


Testing
---

./gradlew -Pq build

No new tests added - the existing JerseyServletModuleTests already cover 
correctness of wiring.


Thanks,

Kevin Sweeney



Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney


 On Sept. 12, 2014, 2:37 p.m., Joshua Cohen wrote:
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java, 
  lines 369-373
  https://reviews.apache.org/r/25593/diff/1/?file=688035#file688035line369
 
  Obviously not related, but should these all use Objects.requireNonNull 
  instead of Preconditions.checkNotNull?

done


- Kevin


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


On Sept. 12, 2014, 2:39 p.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25593/
 ---
 
 (Updated Sept. 12, 2014, 2:39 p.m.)
 
 
 Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This paves the way for ShiroWebModule integration. As an added benefit Guice 
 is now instantiating our servlets for us (no injector.getInstance calls in 
 our code).
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 9d8654b4268a13fc9d29d12f8da2c1aa8fda4b2b 
 
 Diff: https://reviews.apache.org/r/25593/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 No new tests added - the existing JerseyServletModuleTests already cover 
 correctness of wiring.
 
 
 Thanks,
 
 Kevin Sweeney
 




Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney

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

(Updated Sept. 12, 2014, 2:43 p.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.


Changes
---

jcohen's feedback


Repository: aurora


Description
---

This paves the way for ShiroWebModule integration. As an added benefit Guice is 
now instantiating our servlets for us (no injector.getInstance calls in our 
code).


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
9d8654b4268a13fc9d29d12f8da2c1aa8fda4b2b 

Diff: https://reviews.apache.org/r/25593/diff/


Testing
---

./gradlew -Pq build

No new tests added - the existing JerseyServletModuleTests already cover 
correctness of wiring.


Thanks,

Kevin Sweeney



Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Sept. 12, 2014, 9:43 p.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25593/
 ---
 
 (Updated Sept. 12, 2014, 9:43 p.m.)
 
 
 Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This paves the way for ShiroWebModule integration. As an added benefit Guice 
 is now instantiating our servlets for us (no injector.getInstance calls in 
 our code).
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 9d8654b4268a13fc9d29d12f8da2c1aa8fda4b2b 
 
 Diff: https://reviews.apache.org/r/25593/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 No new tests added - the existing JerseyServletModuleTests already cover 
 correctness of wiring.
 
 
 Thanks,
 
 Kevin Sweeney