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 Mon, 25 May 2015 07:03:20 GMT


> On May 20, 2015, 11:47 a.m., Alexander Rojas wrote:
> > src/examples/test_hook_module.cpp, lines 36-38
> > <https://reviews.apache.org/r/34361/diff/1/?file=962951#file962951line36>
> >
> >     This constants are used nowhere but in one method. Is there any reason why they
are not defined in the method itself or at least as static attributes of the TestHook class?
> 
> Colin Williams wrote:
>     There's a mitigating circumstance for these declarations, there's a comment a few
lines up from the change instructing that these should be kept in sync with the values in
src/tests/hook_tests.cpp. Presumably they haven't been pulled into a separate file because
it would make the example less self-contained. Bringing the names and types in sync with the
ones created in hook_tests.cpp, but leaving them where they are.
> 
> Colin Williams wrote:
>     Actually, can anybody tell why these need to be kept in sync? I don't see any place
in the code where these two files are connected.

Well, based on some experimentation, those variables clearly need to be kept in sync. For
the ones I introduced, on the other hand, that doesn't seem to matter. In fact, in the case
of test_hook_module.cpp the values aren't even repeated, so the variable doesn't really seem
necessary.


- Colin


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


On May 25, 2015, 6:59 a.m., Colin Williams wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34361/
> -----------------------------------------------------------
> 
> (Updated May 25, 2015, 6:59 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> 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