Code review comment for lp:~jameinel/bzr/2.0.1-322807-branch-at-root

Revision history for this message
Andrew Bennetts (spiv) wrote :

This seems good. I don't have a convenient Windows install to test it on, but I assume you've already taken care of that :)

I have some minor suggestions, so I'm voting Needs Fixing and marking the overall review as Approved (as that seems to be the closest approximation of bb:tweak).

81 + try:
82 + next_entries = _listdir(current)
83 + except OSError: # enoent

That's a bit odd. Why not actually check that the exc.errno == ENOENT?

87 for look in _listdir(current):

This repeats the listdir we just did. Why not reuse 'next_entries'?

129 + # This path probably does not exist
130 + # We can't test this on Windows, because it has a 'MIN_ABS_PATHLENGTH'
131 + # check...
132 + # self.assertRelpath('foo', '/', '/foo')

Ideally this would be in a separate test method that could do a requireFeature check, I think. Not a big deal.

review: Needs Fixing

« Back to merge proposal