mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Artem Harutyunyan" <ar...@mesosphere.io>
Subject Re: Review Request 36978: MESOS-3142 Refactoring os::shell - patch 1/2
Date Thu, 06 Aug 2015 19:06:11 GMT


> On Aug. 4, 2015, 7:54 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 50
> > <https://reviews.apache.org/r/36978/diff/1/?file=1026032#file1026032line50>
> >
> >     should the variable be called `_cmd`?
> 
> Marco Massenzio wrote:
>     note this is neither a private member, not a constructor arg - it's a temp var: AFAIK
there are no guidelines (apart from the obvious naming it sensibly).
>     Renamed `command`

It does not really matter since you renamed, but there is a guideline for function arguments:

```
We prepend constructor and function arguments with a leading underscore to avoid ambiguity
and / or shadowing:
```


- Artem


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


On Aug. 6, 2015, 11:20 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36978/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 11:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3142
>     https://issues.apache.org/jira/browse/MESOS-3142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactoring os::shell.
> See MESOS-3142 for more details.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp ab767ccc4553cc5f61e4fe1b67110a9b5b32f2bc

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 53f14e1869ed7a6e1ac7cc8a82c558ed77907dc9

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 4b7a7bafe1c64183d021b39cf12523250491f0ee

>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 2556bd428cc8990659e30e804b9c96c1659ef4a1

> 
> Diff: https://reviews.apache.org/r/36978/diff/
> 
> 
> Testing
> -------
> 
> make check
> *Note*: this patch by itself breaks mesos - this only fixes the `stout` part: see also
https://reviews.apache.org/r/36979/
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


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