mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Williams" <lack...@gmail.com>
Subject Re: Review Request 34361: converted hard-coded strings to consts
Date Tue, 09 Jun 2015 18:58:07 GMT


> On June 9, 2015, 6:19 p.m., Niklas Nielsen wrote:
> > Hey Colin!
> > 
> > Thanks for contributing this and I am sorry about the tardy turn-around time.
> > 
> > The crux of the ticket was to consolidate the strings, so we only have to maintain
them one place.
> > With that in mind; instead of inlining the string constants, can we add them to
an appropiate header as 'const char[]'?
> > 
> > @Vinod: Would it make sense to add this to src/tests/mesos.hpp and src/tests/mesos.cpp,
as the label tests are in module, master and slave tests?

It made sense to me to connect the values that were actually related, but it seemed like many
of these strings shared a value only because "foo" and "bar" are really common test values.
I thought it would be misleading to consolidate the strings for which it didn't matter if
they matched, am I missing a connection somewhere?


- Colin


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


On June 8, 2015, 7:05 p.m., Colin Williams wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34361/
> -----------------------------------------------------------
> 
> (Updated June 8, 2015, 7:05 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2637
>     https://issues.apache.org/jira/browse/MESOS-2637
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> converted hard-coded strings to consts
> 
> 
> Diffs
> -----
> 
>   src/tests/hook_tests.cpp 3ffde6d 
>   src/tests/master_tests.cpp ba3858f 
>   src/tests/slave_tests.cpp acae497 
> 
> Diff: https://reviews.apache.org/r/34361/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Williams
> 
>


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