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

login
register
mail settings
Submitter Pierre-Yves David
Date Feb. 11, 2014, 9:32 p.m.
Message ID <2890c2e587627bd14728.1392154374@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/3585/
State Accepted
Commit 170f71061069d5b91324f66530c4e1a9e1974e9e
Headers show

Comments

Pierre-Yves David - Feb. 11, 2014, 9:32 p.m.
# 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.
Olle Lundberg - Feb. 11, 2014, 9:53 p.m.
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
>
Pierre-Yves David - Feb. 12, 2014, 12:37 a.m.
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.

Patch

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