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