> On May 23, 2014, 6:53 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java, line 45 > > <https://reviews.apache.org/r/21849/diff/1/?file=589370#file589370line45> > > > > Mixed final and non-final classes in this file - converge on one?
Dropped the only final class. > On May 23, 2014, 6:53 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java, > > line 36 > > <https://reviews.apache.org/r/21849/diff/1/?file=589372#file589372line36> > > > > Not sure I understand the rationale for protected here. The class was formerly abstract, but included no abstract methods and tripped a rule (which i agree with in the vast majority of cases). I removed abstract, but made the constructor protected to require extension for it to be used. I've added a TODO to change this to a helper class, as it's not really useful as a base class. > On May 23, 2014, 6:53 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java, line 133 > > <https://reviews.apache.org/r/21849/diff/1/?file=589373#file589373line133> > > > > Are these just micro-optimizations? Yup. > On May 23, 2014, 6:53 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java, > > line 145 > > <https://reviews.apache.org/r/21849/diff/1/?file=589380#file589380line145> > > > > This will diverge from SanitizedCronJob Would you prefer this move to a factory function in SanitizedCronJob instead? public static Optional<SanitizedCronJob> fromJob(SanitizedConfiguration job) { ... } > On May 23, 2014, 6:53 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java, > > line 124 > > <https://reviews.apache.org/r/21849/diff/1/?file=589390#file589390line124> > > > > why does this become visible? This trips the unused private method rule [1]. The obvious problem is that PMD is ignorant to guice magic. I decided to follow the rule here as i assume this is a small tax to get the coverage in non-guice places. [1] http://pmd.sourceforge.net/pmd-5.1.1/rules/java/unusedcode.html#UnusedPrivateMethod - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21849/#review43854 ----------------------------------------------------------- On May 23, 2014, 6:56 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21849/ > ----------------------------------------------------------- > > (Updated May 23, 2014, 6:56 a.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > By default, the PMD plugin includes the 'basic' ruleSet. Rulesets are > collections of individual rules, the downside being that they're all or none. > To disable one rule within a ruleset, for example, you need to copy the > ruleset definition XML and edit it. I've done that here with two rulesets, > by minimally editing them (added apache header, and commented out disabled > rules). > > If you're interested, you can see more info at the official ruleset listing > [1]. > > Most of the larger edits made are to adhere to the ConfusingTernary rule [2]. > > I also moved the isCron() function as it tripped a rule, and after a bout of > deja vu i realized it recently suffered a complete copy-paste. It is now > de-cloned and lives in SanitizedConfiguration. > > [1] http://pmd.sourceforge.net/pmd-5.1.1/rules/index.html > [2] > http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ConfusingTernary > > > Diffs > ----- > > build.gradle d22038503a7bf894c335d8a446dbd936abf58516 > config/pmd/design.xml PRE-CREATION > config/pmd/logging-java.xml PRE-CREATION > src/main/java/org/apache/aurora/auth/SessionValidator.java > 311ec8db808230b4fffecc8638de5bb0479a4a2e > src/main/java/org/apache/aurora/scheduler/Driver.java > ab5a300122bca6140cb44cbc7c5be5289696bcf4 > src/main/java/org/apache/aurora/scheduler/DriverFactory.java > db864a070a6277d699841c047f4c3317e4cda7c6 > src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java > 11571635ad87889ba6b49a8a6e2180cb66e60a00 > src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java > bdd8c19add724ba5a4c93b4c7850097431113c48 > src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java > e7b1058bbc3f2b19d6c9c54fe4a04347920aafa8 > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java > e8acf0ad50a891ca98dccb80075b1ce31dc2e8dd > src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java > 54748b24d74e37402a6cfb658a1f1695f86c3ec5 > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java > eb4c2b827ecdc3b680ee614a4486e5aa51755d28 > src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java > 6bb9677bc4c8e075ffa125349574ca6b9bcde0bc > src/main/java/org/apache/aurora/scheduler/base/Query.java > 825318a15c8d588df522027aeab7f8943ca4ae39 > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 8d95c7e45b8461c2db587cd9d9359cbe1f923abc > src/main/java/org/apache/aurora/scheduler/configuration/Resources.java > a2679c3bc13ed85c1488f4bd86659a8db0945648 > > src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java > 78a5da1a7b3d1f9025663ad19de5b47d8608ba30 > src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java > fc02264aff6aa190aa8e29ba9a1e38b1bc802b6a > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java > 64fa0681b908b46f2c46532c1d382d968398155e > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java > 6934828d0c6fad9bddb7161349aa770b92351c30 > src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java > c8b606019da6a3d3ec6b023f609cd0f34dabac80 > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java > cf2a425324d049e40cd1b5772df4771b742e0004 > src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java > f68fa1c7c8bddbf5d07c454d3d1f8dcab391277c > src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java > 77975788138709538810f35237e746c43b6522a4 > src/main/java/org/apache/aurora/scheduler/http/Quotas.java > da020776a32dad334fad13f1c310e3eacee786ae > src/main/java/org/apache/aurora/scheduler/http/Slaves.java > 9fa9818e594b7c6add0048b614586be8275d6825 > src/main/java/org/apache/aurora/scheduler/http/StructDump.java > 823668ff814c26393dd02802183def9b333d1c71 > > src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java > 80f3cac4c310f8ca051add7ae4ed4ab029b5d201 > src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java > e4c215b3f5146602624f89a270f5f8d3f451e52a > src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java > e3b5b041f1a877217914040e63373c4018d23177 > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java > 8560bd1251be98bdcd164cf6532731bf2458b9e4 > src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java > 40bc919fbe409c11734d3c38cdd13422569001b4 > src/main/java/org/apache/aurora/scheduler/storage/JobStore.java > 8e03e268e7796e492e27a230db5f47e16462f3e6 > src/main/java/org/apache/aurora/scheduler/storage/LockStore.java > 3b4427a16f9c5ea7f5444f7516e29f8062b915cd > src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java > 6b9e65304617f26f01a49eb3dfea256b7719cb04 > src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java > d48b89e648335dc820e47562e5f3acfa56e24345 > src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java > 44b04d4b5153648b1b7fddfa9f49b62f8e213d59 > src/main/java/org/apache/aurora/scheduler/storage/Storage.java > 4b33fe5dd8223ff04060de0fe16b1c0759ead956 > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java > 6622216c285eb1c25c7648769a7d7676e8b73bcc > src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java > 4dcc5ae92ca4a059d8b23173785d2aa201158b9c > src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java > 9f8ade9ce45467da7608965f9b111134b5fe19b7 > src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java > 88e83e91ef64836ab184053420927448b92ffb0c > src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java > c5e27a15bf5d4334d566831fd969844e9044f746 > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > c851eeb412b17097ff42abce2b7a42fc1c249013 > > src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java > 041d1f9937672df64864be09bbc4fd2a38b9b1ab > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java > 48b02429b17218f10bfa51db6596ba8f0a243c5f > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java > c3a93d1655437e90b1006a28247ce402a8934ab4 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 88bd20d55b488b30dba70d8c1469e058d1c761bc > > src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java > 2134609769d74755d11b43015bd1d17ad49132ec > > Diff: https://reviews.apache.org/r/21849/diff/ > > > Testing > ------- > > ./gradlew build > > > Thanks, > > Bill Farner > >