merging johns integration to mine..
John Arbash Meinel
john at arbash-meinel.com
Tue Jan 3 03:58:35 GMT 2006
Robert Collins wrote:
> On Mon, 2006-01-02 at 16:22 -0600, John Arbash Meinel wrote:
>
>>Robert Collins wrote:
>>
>>>Hi John,
>>> during the merge I caught some VWS issues :).
>>>
>>>But more importantly you had this:
>>>
>>> def setUp(self):
>>> TestCaseInTempDir.setUp(self)
>>> if not paramiko_loaded:
>>> raise TestSkipped('you must have paramiko to run this test')
>>>
>>>
>>>If the setUp fails, then tearDown is not invoked:
>>>the right way to do this as per our test enhancing spec isn't coded yet,
>>>but if you are tweaking setUp to abort, do so before any base class code
>>>runs:
>>>
>>> def setUp(self):
>>> if not paramiko_loaded:
>>> raise TestSkipped('you must have paramiko to run this test')
>>> TestCaseInTempDir.setUp(self)
>>>
>
>
> Did this one.
>
>
>>>Also, PEP8-:
>>>+ def check_mode(test, path, mode):
>>>+ """On win32 chmod doesn't have any effect,
>>>+ so don't actually check anything
>>>+ """
>>>
>>>
>>>Probably would be better with a docstring like:
>>> def check_mode(test, path, mode):
>>> """Win32 does not have modes - do nothing."""
>>>
>>>But the code looks to me like a if block in a single function would
>>>actually be easier to read.
>
>
> Didn't change this, just giving feedback.
>
>
>>>Other than that, I've merged your integration to my integration.
>>>
>>>Cheers,
>>>Rob
>>>
>>
>>Did you make these changes, or are you requesting that I make them?
>
>
> Both :). Also, I found a bug -for me- with the fancy_rename usage
> against paramiko, possibly because I have an old paramiko - could you
> verify I did not fuck it when I tweaked the merge? (I found this after
> sending the prior message).
>
> Cheers,
> Rob
Looking at your changes to fancy_rename, I think you need this:
except IOError, e:
if e.errno not in (None, errno.ENOENT, errno.ENOTDIR):
raise
Otherwise you will be raising if the target doesn't exist, which is
perfectly valid.
John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060102/b00c2bd1/attachment.pgp
More information about the bazaar
mailing list