jcabrerizo commented on a change in pull request #1265: URL: https://github.com/apache/brooklyn-server/pull/1265#discussion_r735507028
########## File path: core/src/main/java/org/apache/brooklyn/util/core/task/ScheduledTask.java ########## @@ -231,10 +231,12 @@ protected String getActiveTaskStatusString(int verbosity) { @Override public boolean isDone(boolean andTaskNoLongerRunning) { + boolean done = isCancelled() || (maxIterations!=null && maxIterations <= runCount) || (period==null && nextRun!=null && nextRun.isDone()); Review comment: I know you didn't change this but i wonder if `maxIterations <= runCount` should be the opposite: is done if `maxIterations > runCount` ########## File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java ########## @@ -173,46 +174,57 @@ public int compare(Task<?> o1, Task<?> o2) { if (!Objects.equal(o1.isSubmitted(), o2.isSubmitted())) { return o1.isSubmitted() ? -1 : 1; } - - // big pref for top-level tasks (manual operations), where submitter null - int weight = 0; - Task<?> o1s = o1.getSubmittedByTask(); - Task<?> o2s = o2.getSubmittedByTask(); - if ("start".equals(o1.getDisplayName()) ||"start".equals(o2.getDisplayName())) { - weight = 0; + // followed by absolute pref for active items + if (!Objects.equal(o1.isDone(), o2.isDone())) { + return !o1.isDone() ? -1 : 1; Review comment: I'd try not to do doble a negation here: ``` return o1.isDone() ? 1 : -1; ``` ########## File path: utils/common/src/main/java/org/apache/brooklyn/util/guava/Functionals.java ########## @@ -20,6 +20,8 @@ import java.util.concurrent.Callable; +import java.util.regex.Matcher; Review comment: Are these import needed? ########## File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java ########## @@ -173,46 +174,57 @@ public int compare(Task<?> o1, Task<?> o2) { if (!Objects.equal(o1.isSubmitted(), o2.isSubmitted())) { return o1.isSubmitted() ? -1 : 1; } - - // big pref for top-level tasks (manual operations), where submitter null - int weight = 0; - Task<?> o1s = o1.getSubmittedByTask(); - Task<?> o2s = o2.getSubmittedByTask(); - if ("start".equals(o1.getDisplayName()) ||"start".equals(o2.getDisplayName())) { - weight = 0; + // followed by absolute pref for active items + if (!Objects.equal(o1.isDone(), o2.isDone())) { + return !o1.isDone() ? -1 : 1; } - if (!Objects.equal(o1s==null, o2s==null)) - weight += 2*60*60 * (o1s==null ? -1 : 1); - - // pretty big pref for things invoked by other entities - if (context!=null && o1s!=null && o2s!=null) { - boolean o1se = context.equals(BrooklynTaskTags.getContextEntity(o1s)); - boolean o2se = context.equals(BrooklynTaskTags.getContextEntity(o2s)); - if (!Objects.equal(o1se, o2se)) - weight += 10*60 * (o2se ? -1 : 1); + + // followed by absolute pref for things not started yet + if (!Objects.equal(o1.isBegun(), o2.isBegun())) { + return !o1.isBegun() ? -1 : 1; Review comment: same as above -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brooklyn.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org