> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > Did a quick review since I spent some time on working on a fix for 
> > MESOS-3335. It would be great if we wouldn't aggreviate that problem 
> > further.
> > 
> > What seems unclear to me ATM is how alias'ed flags can override each other, 
> > or what the preferred order would be.

There is no override or preference order. Only one of name/alias is allowed. 
Otherwise we throw an error.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp, line 64
> > <https://reviews.apache.org/r/46621/diff/1/?file=1358750#file1358750line64>
> >
> >     Does it makes sense to support multiple aliases?

that was how the original patch was in my branch. but reverted it because i 
didn't have any use cases to use multiple aliases. we can add it later if we 
need it.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp, line 81
> > <https://reviews.apache.org/r/46621/diff/1/?file=1358750#file1358750line81>
> >
> >     We could return a const ref here.

just as an optmization? don't think we do that in our code base.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 
> > 152-171
> > <https://reviews.apache.org/r/46621/diff/1/?file=1358751#file1358751line152>
> >
> >     These suffer from the problem described in MESOS-3335.

yes. but IIUC this issue is not introduced here>? i would rather we fix it in 
one swoop when we fix MESOS-3335.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 
> > 192-200
> > <https://reviews.apache.org/r/46621/diff/1/?file=1358751#file1358751line192>
> >
> >     This also suffers from the problem described in MESOS-3335.

see above.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 
> > 212-216
> > <https://reviews.apache.org/r/46621/diff/1/?file=1358751#file1358751line212>
> >
> >     This also suffers from the problem described in MESOS-3335.

see above.


- Vinod


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


On April 25, 2016, 5:44 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46621/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 5:44 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5271
>     https://issues.apache.org/jira/browse/MESOS-5271
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add alias support for flags.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
> 2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 
> 
> Diff: https://reviews.apache.org/r/46621/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to