Re: Review Request 34361: converted hard-coded strings to consts

2015-08-24 Thread Niklas Nielsen
On June 9, 2015, 6:25 p.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-07-16 Thread Colin Williams
On June 10, 2015, 1:25 a.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-07-16 Thread Niklas Nielsen
On June 9, 2015, 6:25 p.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-15 Thread Niklas Nielsen
On June 9, 2015, 6:25 p.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-15 Thread Colin Williams
On June 10, 2015, 1:25 a.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-11 Thread Niklas Nielsen
On June 9, 2015, 6:25 p.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-10 Thread Ben Mahler
On June 10, 2015, 1:25 a.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-10 Thread Niklas Nielsen
On June 9, 2015, 6:25 p.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-09 Thread Colin Williams
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

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-09 Thread Colin Williams
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

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-26 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/#review85285 --- Patch looks great! Reviews applied: [34361] All tests passed. -

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-25 Thread Colin Williams
--- 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

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-23 Thread Colin Williams
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/ --- (Updated May 23, 2015, 8:02 p.m.) Review request for mesos. Changes ---

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-21 Thread Colin Williams
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

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-21 Thread Colin Williams
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

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-20 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/#review84498 --- src/examples/test_hook_module.cpp

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-20 Thread Colin Williams
On May 20, 2015, 11:47 a.m., Alexander Rojas wrote: This is a very good point, my original change had the values set as local variables, but when I read through the task again it seemed like it was asking for predefined constants. I'll make the changes and put them up during my morning

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-18 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/#review84172 --- src/examples/test_hook_module.cpp

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-18 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/#review84180 --- Patch looks great! Reviews applied: [34361] All tests passed. -

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-18 Thread Colin Williams
On May 18, 2015, 5:48 p.m., Marco Massenzio wrote: src/examples/test_hook_module.cpp, lines 36-38 https://reviews.apache.org/r/34361/diff/1/?file=962951#file962951line36 Thanks for doing this! I'm wondering whether, as these are constants, shouldn't they be in