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