mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 56055: Added validation for 'LAUNCH_NESTED_CONTAINER_SESSION'.
Date Tue, 31 Jan 2017 22:25:05 GMT


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

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();
```


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message