[PATCH 1 of 2 V2] rebase: refactor of error handling code path for rebaseskipobsolete

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Mar 29 14:47:41 EDT 2016



On 03/28/2016 04:54 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1459209028 25200
> #      Mon Mar 28 16:50:28 2016 -0700
> # Node ID 179e9f0018b3d61f16a8c14bb26a9edb8e765be9
> # Parent  30863ca01c6b8f519cf1987565eee88734eb03d6
> rebase: refactor of error handling code path for rebaseskipobsolete
>
> This patch extracts the error handling code path to go in a separate function.
> In the next patch we will able to reuse this logic and avoid duplicated code.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -305,31 +305,9 @@ def rebase(ui, repo, **opts):
>                                                                   rebaseobsrevs,
>                                                                   dest)
>                   rebaseobsskipped = set(obsoletenotrebased)
> -
> -                # Obsolete node with successors not in dest leads to divergence
> -                divergenceok = ui.configbool('experimental',
> -                                             'allowdivergence')
> -                divergencebasecandidates = rebaseobsrevs - rebaseobsskipped
> -
> -                if divergencebasecandidates and not divergenceok:
> -                    divhashes = (str(repo[r])
> -                                 for r in divergencebasecandidates)
> -                    msg = _("this rebase will cause "
> -                            "divergences from: %s")
> -                    h = _("to force the rebase please set "
> -                          "experimental.allowdivergence=True")
> -                    raise error.Abort(msg % (",".join(divhashes),), hint=h)
> -
> -                # - plain prune (no successor) changesets are rebased
> -                # - split changesets are not rebased if at least one of the
> -                # changeset resulting from the split is an ancestor of dest
> -                rebaseset = rebasesetrevs - rebaseobsskipped
> -                if rebasesetrevs and not rebaseset:
> -                    msg = _('all requested changesets have equivalents '
> -                            'or were marked as obsolete')
> -                    hint = _('to force the rebase, set the config '
> -                             'experimental.rebaseskipobsolete to False')
> -                    raise error.Abort(msg, hint=hint)
> +                checkdivergenceandemptyrebase(repo, ui, rebaseobsrevs,
> +                                              rebasesetrevs,
> +                                              rebaseobsskipped)
>
>               result = buildstate(repo, dest, rebaseset, collapsef,
>                                   obsoletenotrebased)
> @@ -709,6 +687,43 @@ def nearestrebased(repo, rev, state):
>       else:
>           return None
>
> +def checkdivergenceandemptyrebase(repo, ui,
> +                                  rebaseobsrevs,
> +                                  rebasesetrevs,
> +                                  rebaseobsskipped):

The nameofthisfunctionisgettingintotheproblematiclylongterritory. We can 
probably get something shorter.

1) This in an internal only function, let's add as "_" prefix,
2) This function is validating the rebase action, other similar function 
start with "check" (you got that right).
3) The function read obsolescence information to do its checks, let's 
add "obs"
4) this function is checking the rebase action lets add "rebase".

New proposal: _checkobsrebase(…)

Feel free to us something else that would make more sense to you (that 
is not 29 char long).

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list