Patchwork [2,of,2] discovery: prefer loop to double-for list comprehension in changegroupsubset

login
register
mail settings
Submitter Kevin Bullock
Date Nov. 24, 2013, 11:38 p.m.
Message ID <40ef143ef1d9749ebf42.1385336284@billings.local>
Download mbox | patch
Permalink /patch/3119/
State Accepted
Commit 01bdccfeb9d98ae85388d06c9c694b946f346edf
Headers show

Comments

Kevin Bullock - Nov. 24, 2013, 11:38 p.m.
# HG changeset patch
# User Kevin Bullock <kbullock@ringworld.org>
# Date 1385336019 21600
#      Sun Nov 24 17:33:39 2013 -0600
# Node ID 40ef143ef1d9749ebf42d3c71170cf2f10f20235
# Parent  4f8e4f2eb8fdfd521d4883d48d9ab6685f64d0a4
discovery: prefer loop to double-for list comprehension in changegroupsubset

The double-for form of list comprehensions gets particularly unreadable
when you throw in an 'if' condition. This expands the only remaining
instance of the double-for syntax in our codebase into a loop.
Augie Fackler - Nov. 27, 2013, 2:57 p.m.
On Sun, Nov 24, 2013 at 05:38:04PM -0600, Kevin Bullock wrote:
> # HG changeset patch
> # User Kevin Bullock <kbullock@ringworld.org>
> # Date 1385336019 21600
> #      Sun Nov 24 17:33:39 2013 -0600
> # Node ID 40ef143ef1d9749ebf42d3c71170cf2f10f20235
> # Parent  4f8e4f2eb8fdfd521d4883d48d9ab6685f64d0a4
> discovery: prefer loop to double-for list comprehension in changegroupsubset
>
> The double-for form of list comprehensions gets particularly unreadable
> when you throw in an 'if' condition. This expands the only remaining
> instance of the double-for syntax in our codebase into a loop.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -2008,8 +2008,11 @@ class localrepository(object):
>              bases = [nullid]
>          # TODO: remove call to nodesbetween.
>          csets, bases, heads = cl.nodesbetween(bases, heads)
> -        bases = [p for n in bases for p in cl.parents(n) if p != nullid]
> -        outgoing = discovery.outgoing(cl, bases, heads)
> +        #bases = [p for n in bs for p in cl.parents(n) if p != nullid]

Patches look righteous, but would you like to remove this
commented-out line before pushing?

> +        discbases = []
> +        for n in bases:
> +            discbases.extend([p for p in cl.parents(n) if p != nullid])
> +        outgoing = discovery.outgoing(cl, discbases, heads)
>          bundler = changegroup.bundle10(self)
>          return self._changegroupsubset(outgoing, bundler, source)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2008,8 +2008,11 @@  class localrepository(object):
             bases = [nullid]
         # TODO: remove call to nodesbetween.
         csets, bases, heads = cl.nodesbetween(bases, heads)
-        bases = [p for n in bases for p in cl.parents(n) if p != nullid]
-        outgoing = discovery.outgoing(cl, bases, heads)
+        #bases = [p for n in bs for p in cl.parents(n) if p != nullid]
+        discbases = []
+        for n in bases:
+            discbases.extend([p for p in cl.parents(n) if p != nullid])
+        outgoing = discovery.outgoing(cl, discbases, heads)
         bundler = changegroup.bundle10(self)
         return self._changegroupsubset(outgoing, bundler, source)