> 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
> 
>

Reply via email to