Patchwork [4,of,7,(push,is,done;,12,more,to,go,for,pull)] push: move outgoing check logic in its own function

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@logilab.fr>
# Date 1391144481 28800
#      Thu Jan 30 21:01:21 2014 -0800
# Node ID 2890c2e587627bd14728ccaf6eacc581ee7a4fa9
# Parent  ee7be5db8f2443a3c97a33b3fe4276d2af7e36fd
push: move outgoing check logic in its own function

Now that every necessary information is held in the `pushoperation` object, we
can extract the part responsible of aborting the push to it's own function.

This changeset is mostly pure code movement. the exception is the fact this
function a value to decide if changeset bundle should be pushed.

On Tue, Feb 11, 2014 at 10:32 PM, <pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> # Date 1391144481 28800
> #      Thu Jan 30 21:01:21 2014 -0800
> # Node ID 2890c2e587627bd14728ccaf6eacc581ee7a4fa9
> # Parent  ee7be5db8f2443a3c97a33b3fe4276d2af7e36fd
> push: move outgoing check logic in its own function
>
> Now that every necessary information is held in the `pushoperation`
> object, we
> can extract the part responsible of aborting the push to it's own function.
>
> This changeset is mostly pure code movement. the exception is the fact this
> function a value to decide if changeset bundle should be pushed.
>
you accidentally a word there:

This changeset is mostly pure code movement. the exception is the fact this
function returns a value to decide if changeset bundle should be pushed.

>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -109,43 +109,11 @@ def push(repo, remote, force=False, revs
>                             commoninc=commoninc, force=pushop.force)
>              pushop.outgoing = outgoing
>              pushop.remoteheads = remoteheads
>              pushop.incoming = inc
>
> -
> -            if not outgoing.missing:
> -                # nothing to push
> -                scmutil.nochangesfound(unfi.ui, unfi, outgoing.excluded)
> -            else:
> -                # something to push
> -                if not pushop.force:
> -                    # if repo.obsstore == False --> no obsolete
> -                    # then, save the iteration
> -                    if unfi.obsstore:
> -                        # this message are here for 80 char limit reason
> -                        mso = _("push includes obsolete changeset: %s!")
> -                        mst = "push includes %s changeset: %s!"
> -                        # plain versions for i18n tool to detect them
> -                        _("push includes unstable changeset: %s!")
> -                        _("push includes bumped changeset: %s!")
> -                        _("push includes divergent changeset: %s!")
> -                        # If we are to push if there is at least one
> -                        # obsolete or unstable changeset in missing, at
> -                        # least one of the missinghead will be obsolete or
> -                        # unstable. So checking heads only is ok
> -                        for node in outgoing.missingheads:
> -                            ctx = unfi[node]
> -                            if ctx.obsolete():
> -                                raise util.Abort(mso % ctx)
> -                            elif ctx.troubled():
> -                                raise util.Abort(_(mst)
> -                                                 % (ctx.troubles()[0],
> -                                                    ctx))
> -                    newbm = pushop.ui.configlist('bookmarks', 'pushing')
> -                    discovery.checkheads(unfi, pushop.remote, outgoing,
> -                                         remoteheads, pushop.newbranch,
> -                                         bool(pushop.incoming), newbm)
> +            if _pushcheckoutgoing(pushop):
>                  _pushchangeset(pushop)
>              _pushsyncphase(pushop)
>              _pushobsolete(pushop)
>          finally:
>              if lock is not None:
> @@ -155,10 +123,49 @@ def push(repo, remote, force=False, revs
>              locallock.release()
>
>      _pushbookmark(pushop)
>      return pushop.ret
>
> +def _pushcheckoutgoing(pushop):
> +    outgoing = pushop.outgoing
> +    unfi = pushop.repo.unfiltered()
> +    if not outgoing.missing:
> +        # nothing to push
> +        scmutil.nochangesfound(unfi.ui, unfi, outgoing.excluded)
> +        return False
> +    # something to push
> +    if not pushop.force:
> +        # if repo.obsstore == False --> no obsolete
> +        # then, save the iteration
> +        if unfi.obsstore:
> +            # this message are here for 80 char limit reason
> +            mso = _("push includes obsolete changeset: %s!")
> +            mst = "push includes %s changeset: %s!"
> +            # plain versions for i18n tool to detect them
> +            _("push includes unstable changeset: %s!")
> +            _("push includes bumped changeset: %s!")
> +            _("push includes divergent changeset: %s!")
> +            # If we are to push if there is at least one
> +            # obsolete or unstable changeset in missing, at
> +            # least one of the missinghead will be obsolete or
> +            # unstable. So checking heads only is ok
> +            for node in outgoing.missingheads:
> +                ctx = unfi[node]
> +                if ctx.obsolete():
> +                    raise util.Abort(mso % ctx)
> +                elif ctx.troubled():
> +                    raise util.Abort(_(mst)
> +                                     % (ctx.troubles()[0],
> +                                        ctx))
> +        newbm = pushop.ui.configlist('bookmarks', 'pushing')
> +        discovery.checkheads(unfi, pushop.remote, outgoing,
> +                             pushop.remoteheads,
> +                             pushop.newbranch,
> +                             bool(pushop.incoming),
> +                             newbm)
> +    return True
> +
>  def _pushchangeset(pushop):
>      """Make the actual push of changeset bundle to remote repo"""
>      outgoing = pushop.outgoing
>      unbundle = pushop.remote.capable('unbundle')
>      # TODO: get bundlecaps from remote
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

On 02/11/2014 01:53 PM, Olle wrote:
>
>
>
> On Tue, Feb 11, 2014 at 10:32 PM, <pierre-yves.david@ens-lyon.org
> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@logilab.fr
>     <mailto:pierre-yves.david@logilab.fr>>
>     # Date 1391144481 28800
>     #      Thu Jan 30 21:01:21 2014 -0800
>     # Node ID 2890c2e587627bd14728ccaf6eacc581ee7a4fa9
>     # Parent  ee7be5db8f2443a3c97a33b3fe4276d2af7e36fd
>     push: move outgoing check logic in its own function
>
>     Now that every necessary information is held in the `pushoperation`
>     object, we
>     can extract the part responsible of aborting the push to it's own
>     function.
>
>     This changeset is mostly pure code movement. the exception is the
>     fact this
>     function a value to decide if changeset bundle should be pushed.
>
> you accidentally a word there:
>
> This changeset is mostly pure code movement. the exception is the fact this
> function returns a value to decide if changeset bundle should be pushed.

good spot. A "returns" is missing here.