> On Nov. 25, 2015, 5:01 a.m., Michael Park wrote:
> > src/master/http.cpp, lines 541-551
> > <https://reviews.apache.org/r/40247/diff/6/?file=1137470#file1137470line541>
> >
> >     As far as its implementation, let's do:
> >     
> >     ```cpp
> >     static Resources removeDisks(Resources resources)
> >     {
> >       foreach (Resource& resource, resources) {
> >         resource.clear_disk();
> >       }
> >       return resources;
> >     }
> >     ```

Is this really better? You C++ guys are so tricky with your value semantics :)

To me, the version in the review makes it more obvious that we are copying the 
input value, mutating the copy, and then returning the copy. Whereas in your 
version, at first glance the reader might think the function modifies its input 
value in place.

What do you think? Happy to make the change if you think it is an improvement.


- Neil


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


On Nov. 23, 2015, 5:06 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40247/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2015, 5:06 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
>     https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added HTTP endpoints for creating and destroying persistent volumes.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
>   src/Makefile.am 8d14ff803249b5b81b696d40d37e013960dee41b 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp d4b1edde98925fd51e056f253758afea779be9ed 
>   src/master/master.cpp d2bc83cd77ae7fe723ccb35a7c1e0b70a04a0d6e 
>   src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
>   src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40247/diff/
> 
> 
> Testing
> -------
> 
> (1) make check, including newly added tests
> 
> (2) Manually created/removed persistent volumes via HTTP endpoints + curl.
> 
> (3) Previewed docs in Github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Reply via email to