Re: Review Request 41090: Second iteration of changes for cmake build on linux.

[Alex Clemmer]

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



src/slave/cmake/FindCurl.cmake (line 18)


This looks like it was copied from `FindApr`, so it seems like it might 
make sense to make the comment reflect that instead.



src/slave/cmake/FindCurl.cmake (line 50)


If there is only one name we're looking for, and we're only using it in 
`find_library`, does it make sense to just remove the variable and put it below?



src/slave/cmake/FindCurl.cmake (lines 108 - 109)


Interestingly, the comment suggests that we're exporting `CURL_LIBS` but 
not `CURL_LIB`, but this seems to be doing the opposite.

I do notice that we do this also in `FindApr.cmake`, too, though, which 
also seems like a mistake (but I don't remember my thought process at the time).

So, it seems like it's worth looking at which we're _actually_ exporting, 
and which we _meant_ to export, and writing comments that reflect those. 
Because, if this comment is to be believed, we are exporting basically the 
wrong variables.


- Alex Clemmer


On Dec. 9, 2015, 3:45 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41090/
> ---
> 
> (Updated Dec. 9, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Second iteration of changes for cmake build on linux.
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/FindCurl.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41090/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 37703: Add docker exec command.

[haosdent huang]

> On Dec. 1, 2015, 3:09 a.m., Timothy Chen wrote:
> > src/tests/containerizer/docker_tests.cpp, line 297
> > 
> >
> > This isn't enough still, when the future is ready doesn't mean the 
> > container is actually running yet.
> > 
> > What you could do is to run docker->inspect(containerName, 
> > Miliseconds(500)) and it will retry inspect until it's running, then you 
> > AWAIT that inspect future.

Thx, let me update.


- haosdent


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


On Nov. 29, 2015, 4:37 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37703/
> ---
> 
> (Updated Nov. 29, 2015, 4:37 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3291
> https://issues.apache.org/jira/browse/MESOS-3291
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add docker exec command.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
>   src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
>   src/tests/containerizer/docker_tests.cpp 
> d9e1345aea0ef9db0e50360e3993ef2708970298 
> 
> Diff: https://reviews.apache.org/r/37703/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41092: Added CMake file for agent executable build.

[Joseph Wu]

> On Dec. 8, 2015, 7:30 p.m., Joseph Wu wrote:
> > src/slave/CMakeLists.txt, line 42
> > 
> >
> > The pre-commit hooks will complain about this line.
> 
> Alex Clemmer wrote:
> Ah, I didn't know this was true. I thought `mesos-style` only ran on 
> `.cpp` files? Is that wrong?

`.cpp` files are linted.  Most other (text) files are checked for whitespace, 
i.e. trailing whitespace, too many blank lines, 1 newline before EOF, etc.


- Joseph


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


On Dec. 8, 2015, 7:43 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> ---
> 
> (Updated Dec. 8, 2015, 7:43 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CMake file for agent executable build.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41092/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>