> On Feb. 18, 2016, 7:23 p.m., Benjamin Bannier wrote: > > src/linux/fs.hpp, line 351 > > <https://reviews.apache.org/r/43718/diff/2/?file=1254441#file1254441line351> > > > > Not sure how exactly this will be used later, but I would much prefer a > > less stringly-typed and more type-safe variant. > > > > What about something like this? > > > > enum class FileSystem { ext2, ext3, ext4 }; > > > > bool supported(FileSystem fileSystem); > > > > This would minimize weird usage like the `notexistingfs` you have in > > your test. > > Jie Yu wrote: > One use case for that is: for overlayfs backend, we need to check if > overlayfs is supported or not on the box. > > in fact, i'm ok with the string type so that we don't have to list all > potential fs type that we want to support. It's a simple utility function to > see if an entry exists in /proc/filesystem. I would rather keeping it simple > and not introducing the enum here.
I agree with Jie on this, let's keep it simple here. - Shuai ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43718/#review119689 ----------------------------------------------------------- On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43718/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2016, 2:02 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4707 > https://issues.apache.org/jira/browse/MESOS-4707 > > > Repository: mesos > > > Description > ------- > > Added fs::supported() function. > > > Diffs > ----- > > src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 > src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e > src/tests/containerizer/fs_tests.cpp > 29e43877612fa151e6f6d79268a7411272a7bfeb > > Diff: https://reviews.apache.org/r/43718/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Shuai Lin > >
