[PATCH 1 of 1] convert: make convert from cvs recognise branch points correctly

Martin Geisler mg at lazybytes.net
Tue Apr 14 03:02:26 CDT 2009


Henrik Stuart <henrik.stuart at edlund.dk> writes:

> # HG changeset patch
> # User Henrik Stuart <henrik.stuart at edlund.dk>
> # Date 1239545362 -7200
> # Node ID bc98742e4eb7c2d8af9568ca80f38c3937cd076b
> # Parent  aece3c9e62f1a685e4cebd3ecf0b22e1c8101ae1
> convert: make convert from cvs recognise branch points correctly

I don't know anything about CVS, so I cannot comment on that. But I
have comments about the codding style, please see below.

> diff --git a/hgext/convert/cvsps.py b/hgext/convert/cvsps.py
> --- a/hgext/convert/cvsps.py
> +++ b/hgext/convert/cvsps.py
> @@ -35,6 +35,7 @@
>          .tags      - list of tags on the file
>          .synthetic - is this a synthetic "file ... added on ..." revision?
>          .mergepoint- the branch that has been merged from (if present in rlog output)
> +        .branchesto- list of branches that occur at this revision
>      '''
>      def __init__(self, **entries):
>          self.__dict__.update(entries)
> @@ -378,6 +379,8 @@
>              # clean up the results and save in the log.
>              store = False
>              e.tags = util.sort([scache(x) for x in tags.get(e.revision, [])])
> +            e.branchesto = [tag for tag in branchmap if len(branchmap[tag].split('.')) > 2 and branchmap[tag].split('.')[-2] == '0' and e.revision == tuple([int(x) for x in branchmap[tag].split('.')[:-2]])]

I'm sorry to bother you but it is normal to keep line lengths below 80
characters or so -- you hit 200 characters above. As an example, you
can take advantage of the grouping provided by the list comprehension.

I tried

               e.branchesto = [tag for tag in branchmap
                               if (len(branchmap[tag].split('.')) > 2
                                   and branchmap[tag].split('.')[-2] == '0'
                                   and e.revision == tuple([int(x) for
                               x in branchmap[tag].split('.')[:-2]]))]

but even that is not very comprehensible. You also evaluate the
branchmap[tag].split('.') expression three times -- I think it would
be better to make a normal loop and evaluate it once per iteration.

-- 
Martin Geisler

VIFF (Virtual Ideal Functionality Framework) brings easy and efficient
SMPC (Secure Multiparty Computation) to Python. See: http://viff.dk/.


More information about the Mercurial-devel mailing list