> On Feb. 6, 2015, 9:09 p.m., Ben Mahler wrote: > > src/tests/persistent_volume_tests.cpp, lines 95-127 > > <https://reviews.apache.org/r/30694/diff/2/?file=852444#file852444line95> > > > > camelCase on these? Unless you want these to be structs that are > > implicitly convertible to Operation and Resource?
Hum, look like in tests, we follow the google style since we are inherienting from ::testing::Test? E.g., CreateSlaveFlags, CreateMasterFlags, etc. > On Feb. 6, 2015, 9:09 p.m., Ben Mahler wrote: > > src/tests/persistent_volume_tests.cpp, lines 82-92 > > <https://reviews.apache.org/r/30694/diff/2/?file=852444#file852444line82> > > > > Why is this needed? Looks like setting the `--resources` flag with > > `"role1"` in the test would be more obvious? Will have more tests coming and all of them need to set --resources. > On Feb. 6, 2015, 9:09 p.m., Ben Mahler wrote: > > src/tests/persistent_volume_tests.cpp, lines 61-80 > > <https://reviews.apache.org/r/30694/diff/2/?file=852444#file852444line61> > > > > Why is this needed? Framework needs to register using 'role1' so that it can receive offer for 'role1'. > On Feb. 6, 2015, 9:09 p.m., Ben Mahler wrote: > > include/mesos/resources.hpp, lines 279-296 > > <https://reviews.apache.org/r/30694/diff/2/?file=852440#file852440line279> > > > > "checkpointing on the slave", right? > > > > I think going forward we'll want to pull out our custom filters from > > the API, checkpointing seems like an internal filter not relevant to > > frameworks. Mind adding a TODO to pull it out? Done. Added a TODO. > On Feb. 6, 2015, 9:09 p.m., Ben Mahler wrote: > > src/tests/persistent_volume_tests.cpp, line 142 > > <https://reviews.apache.org/r/30694/diff/2/?file=852444#file852444line142> > > > > It's not obvious that there's enough disk for the 64MB and 128MB > > volumes below, why not just set `--resources` here? More following tests will use the same structure. Worth pulling it into the test fixture. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30694/#review71497 ----------------------------------------------------------- On Feb. 6, 2015, 7:47 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30694/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2015, 7:47 p.m.) > > > Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. > > > Bugs: MESOS-2100 > https://issues.apache.org/jira/browse/MESOS-2100 > > > Repository: mesos > > > Description > ------- > > Fixed bugs in CREATE/DESTROY operation handlers and added tests. > > > Diffs > ----- > > include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 > src/Makefile.am 93537d17d3c7604a8532ee1453e405630c481ddc > src/master/master.hpp dcfd38ae2fa9e1bd0b477e9719724dba37114d30 > src/master/master.cpp 234bbecc4205036d790b026abd59100eb188f913 > src/tests/persistent_volume_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/30694/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
