[PATCH 5 of 7 🚿🍦] patchbomb: extract 'getbundlemsgs' closure in its own function
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Thu Nov 6 12:54:53 CST 2014
On 11/06/2014 05:58 PM, Martin von Zweigbergk wrote:
> On Thu Nov 06 2014 at 7:47:08 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 1415137703 0
> # Tue Nov 04 21:48:23 2014 +0000
> # Node ID 2d48ca81fd12986424d08a7abd6fda____eaa1d832d7
> # Parent 7a4add054e1090a3aa88464cf1ca54____751b9d3e36
> patchbomb: extract 'getbundlemsgs' 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
> @@ -230,10 +230,35 @@ def _getdescription(repo, defaultbody, s
> msgfile = repo.opener('last-email.txt', 'wb')
> msgfile.write(body)
> msgfile.close()
> return body
>
> +def _getbundlemsgs(repo, sender, bundle, **opts):
> + """Get the full email sending a given bundle
> +
> + This function return a list of "email" tuple (subject, content,
> None).
> + The list is alway one message long in that case.
>
>
> s/alway/always/
true
> Should the reader be assumed to be know that the "None" is for the
> diffstat? (I'm perfectly fine with a "yes"; it was just not clear to me.)
Lets says yes (the user should be happy to have some docstring ;) )
> if patches:
> msgs = getpatchmsgs(patches, opts.get('patchnames'))
> elif bundle:
> - msgs = getbundlemsgs(_getbundle(repo, dest, **opts))
> + bundledata = _getbundle(repo, dest, **opts)
> + bundleopts = opts.copy()
> + bundleopts.pop('bundle', None) # already processed
>
>
> Sorry, I didn't get this part. What happens without the pop()? Why is it
> needed because the function is extracted?
without the pop we have a conflict between the positional bundle
argument and the one in **opts.
I should probably have mentionned that in the commit description.
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list