----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12797/#review23601 -----------------------------------------------------------
jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47503> Too bad they don't have a class representing a Duration in hudson. =/ jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47505> Hm.. what does being disabled entail? Maybe a comment if you happen to know? Should the check() function should bail immediately if disabled? jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47500> I'm a little fuzzy on the wiring here, is this argument coming from the webui? jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47501> pull out a constant for this default? :) jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47504> Does this comment match the code? It looks more like: // If the computer just connected, check back later. Is that what you intended? jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47502> Oh: http://javadoc.jenkins-ci.org/hudson/slaves/RetentionStrategy.html#check(T) Can you add a comment describing that this means re-check in ~1 minute? jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47506> Does c.isOffline() imply c.isIdle()? That's what this code assumes, but I can't tell if it's true from the javadocs. jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java <https://reviews.apache.org/r/12797/#comment47507> 2 spaces? - Ben Mahler On July 21, 2013, 7:34 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12797/ > ----------------------------------------------------------- > > (Updated July 21, 2013, 7:34 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java > e02d3c7d4cd832b855de17e002976db64ec89752 > > Diff: https://reviews.apache.org/r/12797/diff/ > > > Testing > ------- > > mvn hpi:run > > > Thanks, > > Vinod Kone > >
