[PATCH 3 of 7 🚿🍦] patchbomb: extract 'getbundle' closure in its own function

[Pierre-Yves David]

On 11/06/2014 05:58 PM, Martin von Zweigbergk wrote:
> On Thu Nov 06 2014 at 7:46:58 AM Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david at fb.com
>     <mailto:pierre-yves.david at fb.com>>
>     # Date 1415136837 0
>     #      Tue Nov 04 21:33:57 2014 +0000
>     # Node ID 13f98632a2921017b3e5983a0bec73____aeeb79d328
>     # Parent  79339dac40621b463dda037eaf8892____52c2255120
>     patchbomb: extract 'getbundle' closure in its own function
>
>     Keep marching toward the promised land of simplification!
>
>     diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
>     --- a/hgext/patchbomb.py
>     +++ b/hgext/patchbomb.py
>     @@ -185,10 +185,35 @@ def _getpatches(repo, revs, **opts):
>               output = cStringIO.StringIO()
>               cmdutil.export(repo, [r], fp=output,
>                            opts=patch.diffopts(ui, opts))
>               yield output.getvalue().split('\n')
>
>     +def _getbundle(repo, dest, **opts):
>     +    """return a bundle containing changesets missing in "dest"
>     +
>     +    The `opts` keyword-arguments are the same as the one accepted
>     by the
>     +    `bundle` command.
>     +
>     +    The bundle is a returned as a single, in memory, binary blob.
>     +    """
>     +    ui = repo.ui
>
>
> Only used once, so inline?

This is not a "new" function. This is just the extraction of a closure. 
In some sense, it is code movement. In order to keep this changeset as 
simple as possible. I avoid altering the moved code and I'm doing the 
minimal amount of work to get it to keep running. The ui = repo.ui. Is 
used all along this series as a simple way to get a `ui` variable back.

In the same way, a lot of function takes a **opts argument while they 
could have more restricted argument.

I would prefer to do such adjustement in follow up patches (but did not 
spotted them so good catch).

>
>     +    tmpdir = tempfile.mkdtemp(prefix='hg-__em__ail-bundle-')
>     +    tmpfn = os.path.join(tmpdir, 'bundle')
>     +    try:
>     +        commands.bundle(ui, repo, tmpfn, dest, **opts)
>     +        fp = open(tmpfn, 'rb')
>     +        data = fp.read()
>     +        fp.close()
>     +        return data
>     +    finally:
>     +        try:
>     +            os.unlink(tmpfn)
>     +        except OSError:
>     +            pass
>     +        os.rmdir(tmpdir)
>     +
>     +
>
>
> Drop duplicate blank line.

Good catch, I wonder why check-commit.t did not complained about it.

-- 
Pierre-Yves David