bzr on Windows

[Robert Collins]

On Fri, 2005-09-23 at 15:23 +0300, Alexander Belchenko wrote:
> Here is patch against bzr.dev r.1219 (release 0.0.8) for fixing 
> Windows-related issues.
> 
> With this patch on Windows bzr pass 52 tests and fails on test_merge;
> on Cygwin it pass 61 tests and fails on testremotebranch, but I'm report 
> about this aerlier.
> 
> I also make some minor fix in accordance with PEP-8, HACKING guideline 
> from bzr package, and Deadly Sins article from doc/ subdirectory.

Hi, more conflicts I'm afraid, but I've applied all that didn't. If I
was a little less busy, I'd resolve them myself... sorry.

In your patch, you made multiple line docstrings that looked like:
def open_containing(url):
    """Open an existing branch containing url,
       (searches upwards from url).
       """

This is the PEP8 way:
def open_containing(url):
    """Open an existing branch which contains url.

       This probes for a branch at url, and searches upwards from there.
       """

Note that the first line is a sentence which makes sense on its own, and
the blank line before the rest of the docstring.

========================

In bzrlib.osutils where you changed kind_marker, I've not accepted that
part of the patch for now: There was some discussion about where we
should use os.sep and where we should 'normalise' and use '/'
throughout, and until I've reviewed that thread I'm inclined to preserve
the current behaviour.

The change to is_inside seem strange to me on two things. Firstly,
surely you should just replace always ?

if os.sep != "/":
    dir = dir.replace(os.sep, '/')

return fname.startswith(dir)

Or are you planning to support both separators in the input ? That seems
unusual to me - the normal way is to just depend on callers using
os.sep.

And if we depend on that, then we don't need to do a replace call at
all.

==========================

uuidgen commenting - please don't comment things out. Delete them - we
do have a VCS after all :)

==========================

the changes to _get_user_id are also unusual:

-        return (open(os.path.join(config_dir(), "email"))
+        return (file(os.path.join(config_dir(), "email"), "r")

What did you find wrong with the call to open ? "r" is the default mode
for open according to pydoc, and open and file are synonyms.

==========================

Here you don't need the '\':

            raise BzrError("%r doesn't seem to contain " 
                           "a reasonable email address" % e)

Is file - the "string1" and 
              "string2" are joined together by python.

=========================

For docstrings, the closing """ should be on the same line as the
docstring unless it is a multiple line docsting - the local_time_offset
docstring isn't multiple lines, so must be """...""".

=========================

The xml.py changes I've also rejected - we're currently discussing what
to do with our cElementTree imports. 

I'm offline at the moment, so will push this as soon as I'm on line.

Cheers,
Rob


-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050926/4594873e/attachment.pgp