[PATCH 0 of 1] bfiles: new sshstore

[Greg Ward]

On Mon, Nov 9, 2009 at 3:33 PM,  <david.douard at logilab.fr> wrote:
> One other point, I've started to look how to implement the sshstore.verify
> method. And I wonder if we would not better have a basestore class
> implementing the main logic of the tree methods (get, put anf verify) making use
> of low-level methods to do the actual job of getting or putting a file in the store.

Yes, definitely.  I don't really care about non-local verify, because
it'll be so slow.  (Actually, local verify is horribly slow at the
moment.  I did something wrong in there!)  So I'll let you implement
that.  Feel free to do whatever refactoring is required to reduce
duplicate code.

My general philosophy around refactoring is: just get the damn code
working correctly, and don't worry too much about duplication
immediately.  Get the tests passing.  *Then* factor out the common
code.  Then commit.  It's much easier to see common code when it's
right there in front of you than to try to visualize it ahead of time.
 (Another way of stating the same thing: don't write frameworks,
extract them.)

Sometimes, of course, the required refactoring is blindingly obvious.
We probably have such a case here.  So feel free to send a refactoring
patch, then a patch to implement verify() in the other stores.  MQ is
just made for this kind of thing.  It rocks.

> For example, the implementation of get method in the newly added httpstore,
> retriveving the file in a temp location, chacking its integrity before overwriting the
> file in the working directory should done in each and every implementation of a
> bfile store.

Yup.  In this case, I just wrote the stupid code to get the tests
passing, then committed.  The whole refactor thing kinda slipped by.
(I was in a hurry, and I didn't want to cause you conflicts by
modifying sshstore.)  I will happily accept patches to reduce the
duplication.

Greg