> On 十一月 23, 2015, 1:35 a.m., Guangya Liu wrote: > > src/master/http.cpp, line 541 > > <https://reviews.apache.org/r/40247/diff/4/?file=1135083#file1135083line541> > > > > Can you please add some comments here to clarify why need this? > > Guangya Liu wrote: > Another point is that can we set this as a helper function in > resources.cpp? > > Neil Conway wrote: > I considered making removeDisks() a helper method in > resources.cpp/resources.hpp, but I opted against it because resources.hpp is > a quasi-public header (so changing it later has backward compat concerns), > and it wasn't clear to me that this functionality will be useful for other > callers. > > Guangya Liu wrote: > I think adding a new function will not cause backward compat issues, > comments? > > Neil Conway wrote: > Adding a new function will not, but if we want to change the function > later, it might. For example, we should rename `flatten()` because it is a > not a well-chosen name, but it won't be trivial to do so because the > resources API is semi-public.
Yes, changing names in resources.cpp/hpp will cause backward compat issues, for this patch, can we just move removeDisks() to resources.cpp/hpp without changing any existing function names? - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40247/#review107531 ----------------------------------------------------------- On 十一月 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 十一月 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 > >