> On May 14, 2018, 7:26 p.m., Aurora ReviewBot wrote: > > Master (805a53f) is red with this patch. > > ./build-support/jenkins/build.sh > > > > :compileJmhJavaNote: > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java > > uses or overrides a deprecated API. > > Note: Recompile with -Xlint:deprecation for details. > > > > :processJmhResources NO-SOURCE > > :jmhClasses > > :checkstyleJmh > > :checkstyleMain[ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:789:7: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:790:9: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:802:7: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:805:14: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:808:14: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:816:9: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java:123:9: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java:127:16: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java:20:8: > > Unused import - com.google.common.collect.FluentIterable. [UnusedImports] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java:22: > > Wrong order for 'java.util.stream.Collectors' import. Order should be: > > java, javax, scala, com, net, org. Each group should be separated > > by a single blank line. [ImportOrder] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:19: > > Wrong order for 'java.util.List' import. Order should be: java, javax, > > scala, com, net, org. Each group should be separated by a single > > blank line. [ImportOrder] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:25: > > Wrong order for 'java.util.stream.Collectors' import. Order should be: > > java, javax, scala, com, net, org. Each group should be separated > > by a single blank line. [ImportOrder] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:70:11: > > Redundant 'final' modifier. [RedundantModifier] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:70:44: > > '{' is not preceded with whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:100:11: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:109:49: > > '-' is not preceded with whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:109:50: > > '-' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:117:7: > > 'if' is not followed by whitespace. [WhitespaceAround] > > [ant:checkstyle] [ERROR] > > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:127: > > Line is longer than 100 characters (found 101). [LineLength] > > FAILED > > > > FAILURE: Build failed with an exception. > > > > * What went wrong: > > Execution failed for task ':checkstyleMain'. > > > Checkstyle rule violations were found. See the report at: > > > file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html > > > > * Try: > > Run with --stacktrace option to get the stack trace. Run with --info or > > --debug option to get more log output. > > > > * Get more help at https://help.gradle.org > > > > BUILD FAILED in 5m 13s > > 32 actionable tasks: 26 executed, 6 up-to-date > > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry"
Can you clean up the style issues? Also, will this be configurable via the pystachio config? - Santhosh Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66192/#review203093 ----------------------------------------------------------- On May 14, 2018, 7:19 p.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66192/ > ----------------------------------------------------------- > > (Updated May 14, 2018, 7:19 p.m.) > > > Review request for Aurora, David McLaughlin, Jordan Ly, Santhosh Kumar > Shanmugham, and Stephan Erb. > > > Repository: aurora > > > Description > ------- > > Adding support for variable group sizes when executing an update. > > Design doc for this change is here: > https://docs.google.com/document/d/1xGk4ueH8YlmJCk6hQJh85u4to4M1VQD0l630IOchvgY/edit#heading=h.lg3hty82f5cz > > I opted for the path of least resistance with regards to the Thrift changes > as I didn't see any benefit in making the larger changes required to make the > interfaces a bit more flexible. > > Requesting feedback on these changes and the approach from the community > before I proceed. > > Tests will be added after the community approves of the direciton and > approach. > > Note to reviewers: Changes made in ActiveLimitedStrategy.java were made to > move towards getting rid of FluentIterable. I figured since I was touching > that code, it wouldn't hurt to test the Java 8 equivalent of it. I can get > rid of the change here and make it in a separate patch if desired. > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > ef754e32172e7490a47a13e7b526f243ffa3efeb > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 9fc0416086dd3eb2e2f4e8f659da59fcdea2b22b > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 3992aa77fc305adc390a4aaeb1d3939d6241ddbd > > src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java > 855ea9c20788b51695b7eff5ac0970f0d52a9546 > > src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66192/diff/3/ > > > Testing > ------- > > > Thanks, > > Renan DelValle > >