> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, line 24
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line24>
> >
> >     This only works for CentOS 7+ since otherwise we need devtoolset-2, did 
> > you want add a note?

added a note.


> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, line 33
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line33>
> >
> >     Did you want to add a note for < 14.10, based on Cody's compiler matrix?

done


> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, line 77
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line77>
> >
> >     This doesn't seem to work for me:
> >     
> >     ```
> >     $ cat /dev/urandom | tr -dc 'a-z0-9' | fold -w 32 | head -n 1
> >     tr: Illegal byte sequence
> >     o%
> >     ```
> >     
> >     Any reason not to just use `$RANDOM`?
> >     
> >     ```
> >     $ echo $RANDOM
> >     21292
> >     ```

that was taken from https://gist.github.com/earthgecko/3089509. doesn't seem to 
work on BSD based distros though. 

will do a simpler random string

```
mesos-`date +%s`-$RANDOM
```


> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, lines 82-83
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line82>
> >
> >     Should this go before we `build` the image, in case it fails?

not really. if build fails no image will be created.


> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, line 15
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line15>
> >
> >     Are you not concerned about accidentally deleting someone's Dockerfile 
> > here?

there shouldn't be a Dockerfile at the root of Mesos repo. but yea, once ASF 
supports Docker 1.5 we can use custom Dockerfile name.


> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, lines 8-9
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line8>
> >
> >     ".sh"
> >     
> >     Would be nice to remove this dependency, could we get our directory 
> > with:
> >     
> >     ```
> >     MESOS_DIRECTORY=$( cd "$( dirname "$0" )/.." && pwd )
> >     
> >     cd $MESOS_DIRECTORY
> >     ```
> >     
> >     This doesn't affect the parent shell running the script, so you're left 
> > in the same directory after the script finishes. Thoughts?

SGTM. fixed.


> On April 10, 2015, 1:09 a.m., Ben Mahler wrote:
> > support/jenkins_build.sh, lines 11-13
> > <https://reviews.apache.org/r/33045/diff/1/?file=922071#file922071line11>
> >
> >     s/should/must/ ?
> >     
> >     Or s/should be set/is required/

done


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33045/#review79611
-----------------------------------------------------------


On April 9, 2015, 11:47 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33045/
> -----------------------------------------------------------
> 
> (Updated April 9, 2015, 11:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Cody Maloney.
> 
> 
> Bugs: MESOS-2233
>     https://issues.apache.org/jira/browse/MESOS-2233
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> My goal is to replace the miscelleneous Jenkins jobs on ASF with a single 
> multi-configuration job whose configuration lies in the repo. This will make 
> the CI management much simpler.
> 
> 
> Diffs
> -----
> 
>   support/jenkins_build.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33045/diff/
> 
> 
> Testing
> -------
> 
> Tested with a custom job on ASF Jenkins.
> 
> https://builds.apache.org/job/vinod-docker-multi/
> 
> NOTE: Disabled clang on CentOS because of known issue 
> (https://issues.apache.org/jira/browse/MESOS-1385).
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to