Re: Review Request 21951: Enable more PMD rules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/#review44123 --- src/main/java/org/apache/aurora/scheduler/http/Utilization.java https://reviews.apache.org/r/21951/#comment78448 Do we really want to start using Locale everywhere when dealing with string conversion? It might be better to just make it a pre-requisite for the Aurora rather than handling it in code. - Maxim Khutornenko On May 28, 2014, 12:57 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/ --- (Updated May 28, 2014, 12:57 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Similar to previous review [1], the xml files are extracted from the pmd project, with disabled rules commented out. I'm happy to explain the rules behind any changes made, or you can see for yourself by patching, undoing the change, and running the build command. [1] https://reviews.apache.org/r/21849/ Diffs - build.gradle 646cdd2ac7a0beef7df5d02f16b1d37defb0c109 config/pmd/imports.xml PRE-CREATION config/pmd/naming.xml PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 2b97198bea5068fed3753f6e18e0d59115a4cf91 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java d717ae1a71b7b46713683db843bab28111712c44 src/main/java/org/apache/aurora/scheduler/base/Conversions.java 3340a2a7885e79724999b7c87e223a4003124084 src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java 6bfe5ca29f457130aaa8f302d9c7a60e96e93764 src/main/java/org/apache/aurora/scheduler/cron/quartz/Quartz.java c60868e431db3716f3af087ed7de64f448ca95ae src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 3a28a25f0b677468517fbb66797edea7b6e3b1e6 src/main/java/org/apache/aurora/scheduler/http/Utilization.java aed98230d63f7177f522c1440bdd646e9f731d47 src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java 045a37087ef8061937cbaad85945b028cdb63578 src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 515fb24f0f1629ed318a4c3e3ee0b64fce70676a src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java c03c6d114a667bc4a8d142bfd18d9b1529d6d164 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java eb8411021d91b337f984766a8d5ecb6518a38385 src/main/java/org/apache/aurora/scheduler/stats/SlotSizeCounter.java 822721ce7745c9a6c204cf0ae15ac69823760198 src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java fa49e22a40bf2fd216e357869063e31d6d882524 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 10b596b967ccad6e0554f951f44311540aefe698 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2a338e96a843fd6175b304a00fd3ff86de8f1a83 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java e2b70c3d5a669d204019160f87d45cb609dff811 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9b7f6c3755021f2c34dfbffed60265b121c29fdb src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 67d911ed2192211efaa7ec966774cb2536b6ea91 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 16ece207f8cf8f624cf864db798981c639148c36 Diff: https://reviews.apache.org/r/21951/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 21951: Enable more PMD rules.
On May 28, 2014, 4:26 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/http/Utilization.java, line 164 https://reviews.apache.org/r/21951/diff/1/?file=595644#file595644line164 Do we really want to start using Locale everywhere when dealing with string conversion? It might be better to just make it a pre-requisite for the Aurora rather than handling it in code. The problem is that omitting the locale makes the code non-deterministic [1] [2]. It's a semi-rare case, but it seems prudent to make the code behave the same regardless of the default locale. Given how infrequently we're using these functions, i think it's the right thing to do. [1] http://javapapers.com/core-java/javas-tolowercase-has-got-a-surprise-for-you/ [2] http://stackoverflow.com/questions/11063102/using-locales-with-javas-tolowercase-and-touppercase - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/#review44123 --- On May 28, 2014, 12:57 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/ --- (Updated May 28, 2014, 12:57 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Similar to previous review [1], the xml files are extracted from the pmd project, with disabled rules commented out. I'm happy to explain the rules behind any changes made, or you can see for yourself by patching, undoing the change, and running the build command. [1] https://reviews.apache.org/r/21849/ Diffs - build.gradle 646cdd2ac7a0beef7df5d02f16b1d37defb0c109 config/pmd/imports.xml PRE-CREATION config/pmd/naming.xml PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 2b97198bea5068fed3753f6e18e0d59115a4cf91 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java d717ae1a71b7b46713683db843bab28111712c44 src/main/java/org/apache/aurora/scheduler/base/Conversions.java 3340a2a7885e79724999b7c87e223a4003124084 src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java 6bfe5ca29f457130aaa8f302d9c7a60e96e93764 src/main/java/org/apache/aurora/scheduler/cron/quartz/Quartz.java c60868e431db3716f3af087ed7de64f448ca95ae src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 3a28a25f0b677468517fbb66797edea7b6e3b1e6 src/main/java/org/apache/aurora/scheduler/http/Utilization.java aed98230d63f7177f522c1440bdd646e9f731d47 src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java 045a37087ef8061937cbaad85945b028cdb63578 src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 515fb24f0f1629ed318a4c3e3ee0b64fce70676a src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java c03c6d114a667bc4a8d142bfd18d9b1529d6d164 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java eb8411021d91b337f984766a8d5ecb6518a38385 src/main/java/org/apache/aurora/scheduler/stats/SlotSizeCounter.java 822721ce7745c9a6c204cf0ae15ac69823760198 src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java fa49e22a40bf2fd216e357869063e31d6d882524 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 10b596b967ccad6e0554f951f44311540aefe698 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2a338e96a843fd6175b304a00fd3ff86de8f1a83 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java e2b70c3d5a669d204019160f87d45cb609dff811 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9b7f6c3755021f2c34dfbffed60265b121c29fdb src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 67d911ed2192211efaa7ec966774cb2536b6ea91 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 16ece207f8cf8f624cf864db798981c639148c36 Diff: https://reviews.apache.org/r/21951/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 21951: Enable more PMD rules.
On May 28, 2014, 4:26 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/http/Utilization.java, line 164 https://reviews.apache.org/r/21951/diff/1/?file=595644#file595644line164 Do we really want to start using Locale everywhere when dealing with string conversion? It might be better to just make it a pre-requisite for the Aurora rather than handling it in code. Bill Farner wrote: The problem is that omitting the locale makes the code non-deterministic [1] [2]. It's a semi-rare case, but it seems prudent to make the code behave the same regardless of the default locale. Given how infrequently we're using these functions, i think it's the right thing to do. [1] http://javapapers.com/core-java/javas-tolowercase-has-got-a-surprise-for-you/ [2] http://stackoverflow.com/questions/11063102/using-locales-with-javas-tolowercase-and-touppercase I have had an unpleasant experience with locale before where it brought nothing but check tool compliance. Hopefully it will not become a huge burden for us. Otherwise, setting default locale via -Duser JVM flags and checking on startup might be a better alternative. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/#review44123 --- On May 28, 2014, 12:57 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/ --- (Updated May 28, 2014, 12:57 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Similar to previous review [1], the xml files are extracted from the pmd project, with disabled rules commented out. I'm happy to explain the rules behind any changes made, or you can see for yourself by patching, undoing the change, and running the build command. [1] https://reviews.apache.org/r/21849/ Diffs - build.gradle 646cdd2ac7a0beef7df5d02f16b1d37defb0c109 config/pmd/imports.xml PRE-CREATION config/pmd/naming.xml PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 2b97198bea5068fed3753f6e18e0d59115a4cf91 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java d717ae1a71b7b46713683db843bab28111712c44 src/main/java/org/apache/aurora/scheduler/base/Conversions.java 3340a2a7885e79724999b7c87e223a4003124084 src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java 6bfe5ca29f457130aaa8f302d9c7a60e96e93764 src/main/java/org/apache/aurora/scheduler/cron/quartz/Quartz.java c60868e431db3716f3af087ed7de64f448ca95ae src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 3a28a25f0b677468517fbb66797edea7b6e3b1e6 src/main/java/org/apache/aurora/scheduler/http/Utilization.java aed98230d63f7177f522c1440bdd646e9f731d47 src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java 045a37087ef8061937cbaad85945b028cdb63578 src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 515fb24f0f1629ed318a4c3e3ee0b64fce70676a src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java c03c6d114a667bc4a8d142bfd18d9b1529d6d164 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java eb8411021d91b337f984766a8d5ecb6518a38385 src/main/java/org/apache/aurora/scheduler/stats/SlotSizeCounter.java 822721ce7745c9a6c204cf0ae15ac69823760198 src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java fa49e22a40bf2fd216e357869063e31d6d882524 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 10b596b967ccad6e0554f951f44311540aefe698 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2a338e96a843fd6175b304a00fd3ff86de8f1a83 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java e2b70c3d5a669d204019160f87d45cb609dff811 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9b7f6c3755021f2c34dfbffed60265b121c29fdb src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 67d911ed2192211efaa7ec966774cb2536b6ea91 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 16ece207f8cf8f624cf864db798981c639148c36 Diff: https://reviews.apache.org/r/21951/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 21951: Enable more PMD rules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/#review44141 --- Ship it! - David McLaughlin On May 28, 2014, 12:57 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/ --- (Updated May 28, 2014, 12:57 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Similar to previous review [1], the xml files are extracted from the pmd project, with disabled rules commented out. I'm happy to explain the rules behind any changes made, or you can see for yourself by patching, undoing the change, and running the build command. [1] https://reviews.apache.org/r/21849/ Diffs - build.gradle 646cdd2ac7a0beef7df5d02f16b1d37defb0c109 config/pmd/imports.xml PRE-CREATION config/pmd/naming.xml PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 2b97198bea5068fed3753f6e18e0d59115a4cf91 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java d717ae1a71b7b46713683db843bab28111712c44 src/main/java/org/apache/aurora/scheduler/base/Conversions.java 3340a2a7885e79724999b7c87e223a4003124084 src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java 6bfe5ca29f457130aaa8f302d9c7a60e96e93764 src/main/java/org/apache/aurora/scheduler/cron/quartz/Quartz.java c60868e431db3716f3af087ed7de64f448ca95ae src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 3a28a25f0b677468517fbb66797edea7b6e3b1e6 src/main/java/org/apache/aurora/scheduler/http/Utilization.java aed98230d63f7177f522c1440bdd646e9f731d47 src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java 045a37087ef8061937cbaad85945b028cdb63578 src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 515fb24f0f1629ed318a4c3e3ee0b64fce70676a src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java c03c6d114a667bc4a8d142bfd18d9b1529d6d164 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java eb8411021d91b337f984766a8d5ecb6518a38385 src/main/java/org/apache/aurora/scheduler/stats/SlotSizeCounter.java 822721ce7745c9a6c204cf0ae15ac69823760198 src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java fa49e22a40bf2fd216e357869063e31d6d882524 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 10b596b967ccad6e0554f951f44311540aefe698 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2a338e96a843fd6175b304a00fd3ff86de8f1a83 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java e2b70c3d5a669d204019160f87d45cb609dff811 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9b7f6c3755021f2c34dfbffed60265b121c29fdb src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 67d911ed2192211efaa7ec966774cb2536b6ea91 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 16ece207f8cf8f624cf864db798981c639148c36 Diff: https://reviews.apache.org/r/21951/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Review Request 21951: Enable more PMD rules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21951/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Similar to previous review [1], the xml files are extracted from the pmd project, with disabled rules commented out. I'm happy to explain the rules behind any changes made, or you can see for yourself by patching, undoing the change, and running the build command. [1] https://reviews.apache.org/r/21849/ Diffs - build.gradle 646cdd2ac7a0beef7df5d02f16b1d37defb0c109 config/pmd/imports.xml PRE-CREATION config/pmd/naming.xml PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 2b97198bea5068fed3753f6e18e0d59115a4cf91 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java d717ae1a71b7b46713683db843bab28111712c44 src/main/java/org/apache/aurora/scheduler/base/Conversions.java 3340a2a7885e79724999b7c87e223a4003124084 src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java 6bfe5ca29f457130aaa8f302d9c7a60e96e93764 src/main/java/org/apache/aurora/scheduler/cron/quartz/Quartz.java c60868e431db3716f3af087ed7de64f448ca95ae src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 3a28a25f0b677468517fbb66797edea7b6e3b1e6 src/main/java/org/apache/aurora/scheduler/http/Utilization.java aed98230d63f7177f522c1440bdd646e9f731d47 src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java 045a37087ef8061937cbaad85945b028cdb63578 src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 515fb24f0f1629ed318a4c3e3ee0b64fce70676a src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java c03c6d114a667bc4a8d142bfd18d9b1529d6d164 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java eb8411021d91b337f984766a8d5ecb6518a38385 src/main/java/org/apache/aurora/scheduler/stats/SlotSizeCounter.java 822721ce7745c9a6c204cf0ae15ac69823760198 src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java fa49e22a40bf2fd216e357869063e31d6d882524 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 10b596b967ccad6e0554f951f44311540aefe698 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2a338e96a843fd6175b304a00fd3ff86de8f1a83 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java e2b70c3d5a669d204019160f87d45cb609dff811 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9b7f6c3755021f2c34dfbffed60265b121c29fdb src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 67d911ed2192211efaa7ec966774cb2536b6ea91 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 16ece207f8cf8f624cf864db798981c639148c36 Diff: https://reviews.apache.org/r/21951/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
