Merge lp://qastaging/~jelmer/wikkid/dulwich into lp://qastaging/wikkid

ping, anyone?

ping (-:

ping :-)

I'm not sure that I'm really qualified to review this, but cosmic
radiation seems to have flipped a bit in Launchpad's database such that
I have been invited to pass judgement.

[1]

+            try:
+                (mode, sha) = tree[el]
+                if not stat.S_ISDIR(mode):
+                    raise FileExists(
+                        "File %s exists and is not a directory" % el)
+            except KeyError:
+                tree = Tree()
+            else:
+                tree = self.store[sha]

I'm guessing that you're not trying to guard against a KeyError in the
`if not stat.S_ISDIR(...` block, so it probably ought to go in the else:
block.

[2]

+                ret.append(
+                    File(self.store, mode, sha, posixpath.join(directory_path, name), commit_id))

In places you use "/" as the separator. Consider using posixpath.sep
for, I don't know, cleanliness?

[3]

Can you add dulwich as a dependency in setup.py?

[4]

There are a lot of new things here without obvious tests, like the
middleware, but given that there's little other development activity I
hardly want to block for that. In other words, you're very naughty for
not testing things, but there's no punishment, and thanks for the
contribution :)

Thanks for the review, Gavin. Much appreciated! :-)

I'll follow up with some fixes when I get back from vacation.

Thanks, I've now fixed the issues you pointed out.

On 16/01/14 13:06, Jelmer Vernooij wrote:
> Thanks, I've now fixed the issues you pointed out.

Now I have to remember what this is all about and how to run the tests :-)

On Thu, Jan 16, 2014 at 12:18:16AM -0000, Tim Penhey wrote:
> On 16/01/14 13:06, Jelmer Vernooij wrote:
> > Thanks, I've now fixed the issues you pointed out.
>
> Now I have to remember what this is all about and how to run the tests :-)

FWIW I get two failing tests, but those happen under trunk as well. :-)

ping? :)

Guys, I've been pretty crap at following up, but both Jelmer and Gavin have commit rights to trunk :-)

Trunk is owned by ~wikkid. Jelmer, you are good to merge.

Thanks, Tim! Merged.