mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marco Massenzio" <ma...@mesosphere.io>
Subject Re: Review Request 36429: Add filesystem/linux isolator for persistent volumes.
Date Tue, 11 Aug 2015 15:57:06 GMT

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


Wouldn't it be good if we could have some comments as to how this class is supposed to be
used, what does it encapsulate, etc.?

At the very least a URL to a design doc or something?

Also, all the methods are completely undocumented: this means that, whenever anyone will want
in future to use it, they will have to go hunt for the cpp file and reverse engineer the code
(with a large amount of guessing as to the intent for the ambiguous parts) trying to figure
out what each of them does.

(not to mention that it'll be anyone's guess what the methods' arguments are supposed to be).

I'm sure that for those 2-3 people who have spent the last year or so thinking about FSIsolators
this is all obvious; but not so for those who haven't, and even less so for those poor souls
who may want to join the project in future...

- Marco Massenzio


On July 12, 2015, 4:46 a.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36429/
> -----------------------------------------------------------
> 
> (Updated July 12, 2015, 4:46 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved filesystem/linux from review https://reviews.apache.org/r/34135/
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/containerizer.cpp 47d146125dfd4ea909e7ec9d94f41cfa11d035e5

> 
> Diff: https://reviews.apache.org/r/36429/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ian Downes
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message