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

[Martin von Zweigbergk]

On Tue, May 30, 2017 at 1:53 AM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
> On 05/29/2017 11:54 PM, Martin von Zweigbergk wrote:
>>
>>
>>
>> On May 29, 2017 9:50 AM, "Pierre-Yves David"
>> <pierre-yves.david at ens-lyon.org <mailto: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
>>         <mailto: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?
>
>
> I don't think we follow the "mutated" first convention here[1]. On the other
> hand I can think of multiples instance of argument that has "canonical"
> location (eg: ui, repo)

I can't think of any examples of mutable arguments at all (which is
good), so I'm not sure if we're following any convention for them.
I'll switch to your preferred form.

>
> [1] you might have data saying otherwise. I'll be happy to change my mind if
> you have some.
>
> Cheers,
>
> --
> Pierre-Yves David