Re: Review Request 25593: Use a ServletContextListener to configure servlets
--- 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
--- 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
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
--- 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
--- 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