Re: Review Request 56055: Added validation for 'LAUNCH_NESTED_CONTAINER_SESSION'.

[Greg Mann <gr...@mesosphere.io>]

> On Jan. 30, 2017, 9:24 a.m., Jan Schlicht wrote:
> > src/tests/slave_validation_tests.cpp, line 282
> > <https://reviews.apache.org/r/56055/diff/1/?file=1618348#file1618348line282>
> >
> >     Indent with 4 spaces.
> 
> Greg Mann wrote:
>     Unfortunately, I think our style guide is ambiguous on this point? And it seems that within the Mesos codebase we see about equal numbers of both cases, indenting 2 and indenting 4 spaces after a linebreak like this. A little playing around with `clang-format` seems to suggest that it prefers indenting 2 spaces in this case. I don't have a strong opinion, happy to do whatever is the "right" thing here. Local consistency is probably the most important, and since all such occurrences in this file are part of the new tests added in this review chain, I can set them all to be either 2 or 4.
> 
> Greg Mann wrote:
>     I also changed the newline pattern for one of these occurrences to something I prefer a bit more (no opinion on 2 vs. 4 spaces in the indent, I'm fine with either):
>     ```
>     Environment::Variable* variable = launch
>       ->mutable_command()
>       ->mutable_environment()
>       ->mutable_variables()
>       ->Add();
>     ```

I'm gonna drop this issue for now; if there's a case to be made for 4 spaces, I'm happy to change it.


- Greg


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


On Jan. 31, 2017, 10:19 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56055/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2017, 10:19 p.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Vinod Kone.
> 
> 
> Bugs: MESOS-6887
>     https://issues.apache.org/jira/browse/MESOS-6887
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds a validation test for the
> `LAUNCH_NESTED_CONTAINER_SESSION` call.
> 
> 
> Diffs
> -----
> 
>   src/tests/slave_validation_tests.cpp 5de771114982751e7796f55dcacd4384c6989efb 
> 
> Diff: https://reviews.apache.org/r/56055/diff/
> 
> 
> Testing
> -------
> 
> `bin/mesos-tests.sh --gtest_filter="*Validation*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>