Re: Review Request 39944: Add prefix option for os::environment.

2015-11-06 Thread haosdent huang
> On Nov. 5, 2015, 4:40 p.m., Timothy Chen wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/environment.hpp, line 34 > > > > > > Where do we need to use this now? > > I rather not add regex if we

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-06 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/#review105444 ---

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-06 Thread haosdent huang
> On Nov. 6, 2015, 4:22 p.m., Timothy Chen wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/environment.hpp, line 34 > > > > > > I'm not opposed to include this, I just hope we include some context > >

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/ --- (Updated Nov. 5, 2015, 8:25 a.m.) Review request for mesos, Ben Mahler, Jojy

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-05 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/#review105256 ---

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread haosdent huang
> On Nov. 4, 2015, 6:28 p.m., Jojy Varghese wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/environment.hpp, line 25 > > > > > > Do you intend to copy the string each time? Or a const string& will do?

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/ --- (Updated Nov. 5, 2015, 3:18 a.m.) Review request for mesos, Ben Mahler, Jojy

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread haosdent huang
> On Nov. 4, 2015, 6:58 p.m., Timothy Chen wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/environment.hpp, line 25 > > > > > > Why is this needed? > > And also we prefer Option rather than an

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/#review105114 ---

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/#review105110 --- I would suggest you add tests for this. - Jojy Varghese On Nov.

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/#review105109 ---

Re: Review Request 39944: Add prefix option for os::environment.

2015-11-04 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39944/#review105113 ---