> On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/pom.xml, line 11 > > <https://reviews.apache.org/r/12033/diff/1/?file=310237#file310237line11> > > > > Should we lower the version number? 0.1?
done. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java, > > lines 16-22 > > <https://reviews.apache.org/r/12033/diff/1/?file=310243#file310243line16> > > > > Can you fix up the indenting in this file? Using my eclipse's default indentation for java. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java, > > lines 51-64 > > <https://reviews.apache.org/r/12033/diff/1/?file=310243#file310243line51> > > > > Can you clean this up? Whoever wrote this, just copied it line by line from EC2 plugin. I formatted it fwiw. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java, > > lines 67-69 > > <https://reviews.apache.org/r/12033/diff/1/?file=310243#file310243line67> > > > > I don't understand this comment. also copied from ec2 plugin. i added that it is for launching a slave agent. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java, > > lines 79-82 > > <https://reviews.apache.org/r/12033/diff/1/?file=310243#file310243line79> > > > > @Override on same line, I'd make the whole function one line, but I > > don't know how you feel about that stylistically Eclipse default java formatting, likes asis. So, I'm gonna leave it asis for consistency. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java, > > lines 85-89 > > <https://reviews.apache.org/r/12033/diff/1/?file=310243#file310243line85> > > > > Can you move this up to the top? and is disabled final? pulled up. not sure what this is doing. again from ec2. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java, line 47 > > <https://reviews.apache.org/r/12033/diff/1/?file=310244#file310244line47> > > > > Why did we need this again, debugging? If so, s/mesos slave/MesosSlave/ Think so. Fixed. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java, line 72 > > <https://reviews.apache.org/r/12033/diff/1/?file=310244#file310244line72> > > > > Instance id == node name? Yup. IIRC, node names are unique. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/resources/index.jelly, line 5 > > <https://reviews.apache.org/r/12033/diff/1/?file=310246#file310246line5> > > > > My memory is blurry, what did we use this for? Description of the plugin. Updated. > On June 21, 2013, 8:38 p.m., Ben Mahler wrote: > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java, line 54 > > <https://reviews.apache.org/r/12033/diff/1/?file=310244#file310244line54> > > > > are these functions overrides? If so, can you add the @Override > > annotation? doesn't look like it. made getInstanceId() private though, since it is only called from within this class. ? ~/workspace/apache/mesos git:(954cd15) ? grep "terminate(" jenkins jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosComputer.java:25: getNode().terminate(); jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosComputerLauncher.java:88: public void terminate() { jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:60: public void terminate() { jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:70: ((MesosComputerLauncher) launcher).terminate(); jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:85: terminate(); ? ~/workspace/apache/mesos git:(954cd15) ? grep "getInstanceId" jenkins jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:74: + getInstanceId(), e); jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:78: public String getInstanceId() { jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:83: LOGGER.info("Mesos instance idle time expired: " + getInstanceId() ? ~/workspace/apache/mesos git:(954cd15) ? grep "idleTimeout" jenkins jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java:65: c.getNode().idleTimeout(); jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java:82: public void idleTimeout() { - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12033/#review22269 ----------------------------------------------------------- On June 21, 2013, 9:34 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12033/ > ----------------------------------------------------------- > > (Updated June 21, 2013, 9:34 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Jiang Yan Xu. > > > Description > ------- > > This is the initial commit of the Jenkins scheduler. Basically all the code > and proper pom. > > NOTE: Not yet integrated into our build tool chain. > > > This addresses bug MESOS-506. > https://issues.apache.org/jira/browse/MESOS-506 > > > Diffs > ----- > > jenkins/pom.xml PRE-CREATION > jenkins/src/main/java/org/jenkinsci/plugins/mesos/JenkinsScheduler.java > PRE-CREATION > jenkins/src/main/java/org/jenkinsci/plugins/mesos/Mesos.java PRE-CREATION > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosCloud.java > PRE-CREATION > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosComputer.java > PRE-CREATION > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosComputerLauncher.java > PRE-CREATION > > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosRetentionStrategy.java > PRE-CREATION > jenkins/src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java > PRE-CREATION > jenkins/src/main/java/org/jenkinsci/plugins/mesos/TaskTemplate.java > PRE-CREATION > jenkins/src/main/resources/index.jelly PRE-CREATION > > jenkins/src/main/resources/org/jenkinsci/plugins/mesos/MesosCloud/computerSet.jelly > PRE-CREATION > > jenkins/src/main/resources/org/jenkinsci/plugins/mesos/MesosCloud/config.jelly > PRE-CREATION > > jenkins/src/main/resources/org/jenkinsci/plugins/mesos/MesosCloud/help-master.html > PRE-CREATION > > jenkins/src/main/resources/org/jenkinsci/plugins/mesos/MesosSlave/configure-entries.jelly > PRE-CREATION > > jenkins/src/main/resources/org/jenkinsci/plugins/mesos/TaskTemplate/config.jelly > PRE-CREATION > > Diff: https://reviews.apache.org/r/12033/diff/ > > > Testing > ------- > > MESOS_NATIVE_LIBRARY=/Users/vinod/workspace/apache/mesos/build/src/.libs/libmesos.dylib > JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.7.0_07.jdk/Contents/Home > mvn hpi:run > > > Thanks, > > Vinod Kone > >
