[PATCH 1 of 8] rebase: explicitly tests for None

[Augie Fackler]

On Thu, Mar 16, 2017 at 09:35:12AM -0700, Gregory Szorc wrote:
> On Thu, Mar 16, 2017 at 4:28 AM, Pierre-Yves David <
> pierre-yves.david at ens-lyon.org> wrote:
>
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> > # Date 1489615423 25200
> > #      Wed Mar 15 15:03:43 2017 -0700
> > # Node ID 6c6d8c0cc67de1ecd744a30889419510aa2064d2
> > # Parent  361bccce566afe250c8d258f770a908713b2d13f
> > # EXP-Topic mutable-default
> > # Available At https://www.mercurial-scm.org/
> > repo/users/marmoute/mercurial/
> > #              hg pull https://www.mercurial-scm.org/
> > repo/users/marmoute/mercurial/ -r 6c6d8c0cc67d
> > rebase: explicitly tests for None
> >
> > Changeset 361bccce566a removed the mutable default value, but did not
> > explicitly
> > tested for None. Such implicit checking can introduce semantic and
> > performance
> > issue. We move to an explicit check for None as recommended by PEP8:
> >
> > https://www.python.org/dev/peps/pep-0008/#programming-recommendations
>
>
> I generally disagree with this series on the grounds that it doesn't matter
> in these use cases. Yes, there are times where this matters for performance
> or because we're leveraging dynamic programming to pass in different types
> that can all be empty. These are not those times.
>
> Also, if others insist on the "is None" check, I prefer the inline form
> (`revf = revf if revf is not None else []`) because I don't like wasting
> lines on type validation... because Python.

It's a minor change, but on the whole it's more idiomatic, so I'm in
favor of it. Queued (I don't object to the one-line form, but given
that this series is here and done, I'm too lazy to re-do all the
work.)

>
>
> >
> >
> > diff --git a/hgext/rebase.py b/hgext/rebase.py
> > --- a/hgext/rebase.py
> > +++ b/hgext/rebase.py
> > @@ -725,7 +725,8 @@ def _definesets(ui, repo, destf=None, sr
> >                  destspace=None):
> >      """use revisions argument to define destination and rebase set
> >      """
> > -    revf = revf or []
> > +    if revf is None:
> > +        revf = []
> >
> >      # destspace is here to work around issues with `hg pull --rebase` see
> >      # issue5214 for details
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel