[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Monotone-devel] Re: patch: monotone source for mercurial convert extens
From: |
Matt Mackall |
Subject: |
[Monotone-devel] Re: patch: monotone source for mercurial convert extension |
Date: |
Sat, 02 Feb 2008 16:10:23 -0600 |
On Sat, 2008-02-02 at 19:39 +0100, Mikkel Fahnøe Jørgensen wrote:
> Here is an export from monotone to mercurial.
>
> It is based on the mecurial convert extension in hgext/convert.
> It's my first Python program, so ...
Always makes me smile when people use Mercurial to try writing some
Python.
Your patch has some line wrap damage but mostly looks sane.
> Tested with monotone automation interface v. 6.0, monotone version
> 0.38 on OS-X 10.4 Intel and current mercurial source tip.
>
> examples:
>
> hg convert mydb.mtn mynewdb.hg
> hg convert --authors myauthormappingfile --datesort mydb.mtn mynewdb.hg
> hg convert --debug mydb.mtn mynewdb.hg
>
> If this patch is accepted for mercurial, I wouldn't mind if a monotone
> core developer took over maintainance, since the automation interface
> seems to change occasionally.
Hmmm, that's a little worrisome.
A few style pointers:
> +# monotone support for the convert extension
> +
> +import os
> +import re
> +import time
Put these all on one line.
> +from mercurial import util
> +
> +from common import NoRepo, commit, converter_source, checktool
> +
> +class monotone_source(converter_source):
> + def __init__(self, ui, path=None, rev=None):
> + converter_source.__init__(self, ui, path, rev)
> +
> + self.ui = ui
> + self.path = path
> +
> +
One line of whitespace is plenty.
> + # regular expressions for parsing monotone output
> +
> + space = r'\s*'
> + name = r'\s+"((?:[^"]|\\")*)"\s*'
> + value = name
> + revision = r'\s+\[(\w+)\]\s*'
> + lines = r'(?:.|\n)+'
> +
> + self.dir_re = re.compile(space + "dir" + name)
> + self.file_re = re.compile(space + "file" + name +
> "content" + revision)
> + self.add_file_re = re.compile(space + "add_file" + name +
> "content" + revision)
> + self.patch_re = re.compile(space + "patch" + name +
> "from" + revision + "to" + revision)
> + self.rename_re = re.compile(space + "rename" + name + "to" +
> name)
> + self.tag_re = re.compile(space + "tag" + name +
> "revision" + revision)
> + self.cert_re = re.compile(lines + space + "name" + name +
> "value" + value)
I'm generally not a fan of making things line up like this..
> + norepo = NoRepo("%s does not look like a monotone repo" % path)
> + if not os.path.exists(path):
> + raise norepo
Why not just raise NoRepo("...")?
> + &#-1;&#-1; try :
No space before the colon, please.
--
Mathematics is the supreme nostalgia of our time.