[PATCH 3 of 8] hidden: change _domainancestors() to _revealancestors()

[Martin von Zweigbergk]

On May 29, 2017 9:50 AM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
wrote:



On 05/28/2017 08:15 AM, Martin von Zweigbergk wrote:

> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz at google.com>
> # Date 1495945026 25200
> #      Sat May 27 21:17:06 2017 -0700
> # Node ID aef3194855e763ecced80c2df6bd9672207aefbc
> # Parent  a0434758fd9a95ea3807ff4766eaedff6852c628
> hidden: change _domainancestors() to _revealancestors()
>
> This change makes the function will actually reveal the ancestors by
> removing them from the hidden set. This prepares for further
> simplification.
>

That change looks good to me but for a small feedback on the function
signature.


diff --git a/mercurial/repoview.py b/mercurial/repoview.py
> --- a/mercurial/repoview.py
> +++ b/mercurial/repoview.py
> @@ -59,9 +59,11 @@
>                  break
>      return blockers
>
> -def _domainancestors(pfunc, revs, domain):
> -    """return ancestors of 'revs' within 'domain'
> +def _revealancestors(hidden, pfunc, revs, domain):
>

small nit: other functions that use "pfunc" always takes it as first
parameter. I would rather see "pfunc" remains the first parameters here too.


'hidden' gets mutated, and mutated parameters are usually first, I think
(including, but probably not limited to, 'self'). Don't you think that
should trump the pfunc-first convention?



-- 
Pierre-Yves David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170529/ddfc954e/attachment.html>