Code review comment for lp:~sylvain-pineau/checkbox-core/bug983035

Nice work, I'm very pleased about removing the create_from_* class methods and using None instead of an empty File instance! First, I don't think this logic is the same as before:

324 - if (self._user_enable
325 - and not self._openPath(path, self._file, lock=self._lock)):
326 + self._file = self._openPath(path, lock=self._lock)
327 + if (self._user_enable and not self._file):

Before, _openPath was only called if _user_enable is True. Now, _openPath is always called. Second, I think this code is a bit ambiguous because self._writer is not expected to be boolean:

141 + return self._writer == True

Even though your code probably works, I think it's more obvious to say something like: return self._writer is not None. Last, but not least, what would you think about changing Fifo.create_fifo to just be a function, instead of a class method, that would return a reader and writer tuple?