Patchwork repoview: clarify that we want to keep the graph connected while filtering

login
register
mail settings
Submitter via Mercurial-devel
Date May 27, 2017, 7:02 a.m.
Message ID <0c1c0e998fe248978ebb.1495868542@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/20963/
State Accepted
Headers show

Comments

via Mercurial-devel - May 27, 2017, 7:02 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1495868528 25200
#      Sat May 27 00:02:08 2017 -0700
# Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
# Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
repoview: clarify that we want to keep the graph connected while filtering

The word "consistent" was unclear to me -- there are so many
dimensions in which things can consistent.
Pierre-Yves David - May 27, 2017, 10:40 a.m.
On 05/27/2017 09:02 AM, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495868528 25200
> #      Sat May 27 00:02:08 2017 -0700
> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
> repoview: clarify that we want to keep the graph connected while filtering
>
> The word "consistent" was unclear to me -- there are so many
> dimensions in which things can consistent.

I've fine with the direction of the documentation update. However we 
want the set both connected and rooted to nullid. The nullid bits should 
appears somewhere in my opinion

> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
> --- a/mercurial/repoview.py
> +++ b/mercurial/repoview.py
> @@ -54,9 +54,9 @@
>  def _getstatichidden(repo):
>      """Revision to be hidden (disregarding dynamic blocker)
>
> -    To keep a consistent graph, we cannot hide any revisions with
> +    To keep the graph connected, we cannot hide any revisions with
>      non-hidden descendants. This function computes the set of
> -    revisions that could be hidden while keeping the graph consistent.
> +    revisions that could be hidden while keeping the graph connected.
>
>      A second pass will be done to apply "dynamic blocker" like bookmarks or
>      working directory parents.
via Mercurial-devel - May 28, 2017, 6:24 a.m.
On Sat, May 27, 2017 at 3:40 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 05/27/2017 09:02 AM, Martin von Zweigbergk via Mercurial-devel wrote:
>>
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1495868528 25200
>> #      Sat May 27 00:02:08 2017 -0700
>> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
>> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
>> repoview: clarify that we want to keep the graph connected while filtering
>>
>> The word "consistent" was unclear to me -- there are so many
>> dimensions in which things can consistent.

Oops, typo there. Reviewer, please add a "be" before "consistent" in
flight if you don't mind.

>
>
> I've fine with the direction of the documentation update. However we want
> the set both connected and rooted to nullid. The nullid bits should appears
> somewhere in my opinion

It didn't before either, so I think this is still an improvement.

(And no, unless others tell me that "consistent" implied "rooted to
nullid" more than "connected" does, I refuse to believe that that was
a common interpretation.)
Pierre-Yves David - May 28, 2017, 4:23 p.m.
On 05/28/2017 08:24 AM, Martin von Zweigbergk wrote:
> On Sat, May 27, 2017 at 3:40 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>> On 05/27/2017 09:02 AM, Martin von Zweigbergk via Mercurial-devel wrote:
>>>
>>> # HG changeset patch
>>> # User Martin von Zweigbergk <martinvonz@google.com>
>>> # Date 1495868528 25200
>>> #      Sat May 27 00:02:08 2017 -0700
>>> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
>>> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
>>> repoview: clarify that we want to keep the graph connected while filtering
>>>
>>> The word "consistent" was unclear to me -- there are so many
>>> dimensions in which things can consistent.
>
> Oops, typo there. Reviewer, please add a "be" before "consistent" in
> flight if you don't mind.
>
>>
>>
>> I've fine with the direction of the documentation update. However we want
>> the set both connected and rooted to nullid. The nullid bits should appears
>> somewhere in my opinion
>
> It didn't before either, so I think this is still an improvement.
>
> (And no, unless others tell me that "consistent" implied "rooted to
> nullid" more than "connected" does, I refuse to believe that that was
> a common interpretation.)

Well consistent was used for "make sense for what we'll use it for" 
which includes "no changeset with missing parent" equivalent to "rooted 
in nullid)".

However, this is not too important I do not have a too strong opinion. 
If other people find the updated version clearer, we should make that 
change.

Cheers,
Jun Wu - May 28, 2017, 6:43 p.m.
I like the change. It made me understand what is "consistent" instantly.
I also don't think "nullid" clarification should block this patch.

Excerpts from Martin von Zweigbergk's message of 2017-05-27 00:02:22 -0700:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495868528 25200
> #      Sat May 27 00:02:08 2017 -0700
> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
> repoview: clarify that we want to keep the graph connected while filtering
> 
> The word "consistent" was unclear to me -- there are so many
> dimensions in which things can consistent.
> 
> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
> --- a/mercurial/repoview.py
> +++ b/mercurial/repoview.py
> @@ -54,9 +54,9 @@
>  def _getstatichidden(repo):
>      """Revision to be hidden (disregarding dynamic blocker)
>  
> -    To keep a consistent graph, we cannot hide any revisions with
> +    To keep the graph connected, we cannot hide any revisions with
>      non-hidden descendants. This function computes the set of
> -    revisions that could be hidden while keeping the graph consistent.
> +    revisions that could be hidden while keeping the graph connected.
>  
>      A second pass will be done to apply "dynamic blocker" like bookmarks or
>      working directory parents.
Augie Fackler - May 28, 2017, 9:29 p.m.
> On May 27, 2017, at 3:02 AM, Martin von Zweigbergk via Mercurial-devel <mercurial-devel@mercurial-scm.org> wrote:
> 
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495868528 25200
> #      Sat May 27 00:02:08 2017 -0700
> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
> repoview: clarify that we want to keep the graph connected while filtering

Queued this, thanks

(I’m not sure what nullid bit I would add, so it might be worth a follow-up from someone to add that.)

> 
> The word "consistent" was unclear to me -- there are so many
> dimensions in which things can consistent.
> 
> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
> --- a/mercurial/repoview.py
> +++ b/mercurial/repoview.py
> @@ -54,9 +54,9 @@
> def _getstatichidden(repo):
>     """Revision to be hidden (disregarding dynamic blocker)
> 
> -    To keep a consistent graph, we cannot hide any revisions with
> +    To keep the graph connected, we cannot hide any revisions with
>     non-hidden descendants. This function computes the set of
> -    revisions that could be hidden while keeping the graph consistent.
> +    revisions that could be hidden while keeping the graph connected.
> 
>     A second pass will be done to apply "dynamic blocker" like bookmarks or
>     working directory parents.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - May 28, 2017, 9:31 p.m.
> On May 28, 2017, at 5:29 PM, Augie Fackler <raf@durin42.com> wrote:
> 
> 
>> On May 27, 2017, at 3:02 AM, Martin von Zweigbergk via Mercurial-devel <mercurial-devel@mercurial-scm.org> wrote:
>> 
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1495868528 25200
>> #      Sat May 27 00:02:08 2017 -0700
>> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
>> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
>> repoview: clarify that we want to keep the graph connected while filtering
> 
> Queued this, thanks
> 
> (I’m not sure what nullid bit I would add, so it might be worth a follow-up from someone to add that.)

Bah, this no longer applies, and the function has since been renamed. Can you do a resend to update the docstring on the new function to describe what consistency means?

Thanks!

> 
>> 
>> The word "consistent" was unclear to me -- there are so many
>> dimensions in which things can consistent.
>> 
>> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>> --- a/mercurial/repoview.py
>> +++ b/mercurial/repoview.py
>> @@ -54,9 +54,9 @@
>> def _getstatichidden(repo):
>>    """Revision to be hidden (disregarding dynamic blocker)
>> 
>> -    To keep a consistent graph, we cannot hide any revisions with
>> +    To keep the graph connected, we cannot hide any revisions with
>>    non-hidden descendants. This function computes the set of
>> -    revisions that could be hidden while keeping the graph consistent.
>> +    revisions that could be hidden while keeping the graph connected.
>> 
>>    A second pass will be done to apply "dynamic blocker" like bookmarks or
>>    working directory parents.
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
via Mercurial-devel - May 30, 2017, 5:25 a.m.
On Sun, May 28, 2017 at 2:31 PM, Augie Fackler <raf@durin42.com> wrote:
>
>> On May 28, 2017, at 5:29 PM, Augie Fackler <raf@durin42.com> wrote:
>>
>>
>>> On May 27, 2017, at 3:02 AM, Martin von Zweigbergk via Mercurial-devel <mercurial-devel@mercurial-scm.org> wrote:
>>>
>>> # HG changeset patch
>>> # User Martin von Zweigbergk <martinvonz@google.com>
>>> # Date 1495868528 25200
>>> #      Sat May 27 00:02:08 2017 -0700
>>> # Node ID 0c1c0e998fe248978ebbfa9149f3e3c107fb276d
>>> # Parent  4c4d91908492d7474a4f486e9c2a4922f721ddfe
>>> repoview: clarify that we want to keep the graph connected while filtering
>>
>> Queued this, thanks
>>
>> (I’m not sure what nullid bit I would add, so it might be worth a follow-up from someone to add that.)
>
> Bah, this no longer applies, and the function has since been renamed. Can you do a resend to update the docstring on the new function to describe what consistency means?

I didn't even realize it, but my other repoview series refactors the
whole function away, making this patch obsolete.

>
> Thanks!
>
>>
>>>
>>> The word "consistent" was unclear to me -- there are so many
>>> dimensions in which things can consistent.
>>>
>>> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>>> --- a/mercurial/repoview.py
>>> +++ b/mercurial/repoview.py
>>> @@ -54,9 +54,9 @@
>>> def _getstatichidden(repo):
>>>    """Revision to be hidden (disregarding dynamic blocker)
>>>
>>> -    To keep a consistent graph, we cannot hide any revisions with
>>> +    To keep the graph connected, we cannot hide any revisions with
>>>    non-hidden descendants. This function computes the set of
>>> -    revisions that could be hidden while keeping the graph consistent.
>>> +    revisions that could be hidden while keeping the graph connected.
>>>
>>>    A second pass will be done to apply "dynamic blocker" like bookmarks or
>>>    working directory parents.
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>

Patch

diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -54,9 +54,9 @@ 
 def _getstatichidden(repo):
     """Revision to be hidden (disregarding dynamic blocker)
 
-    To keep a consistent graph, we cannot hide any revisions with
+    To keep the graph connected, we cannot hide any revisions with
     non-hidden descendants. This function computes the set of
-    revisions that could be hidden while keeping the graph consistent.
+    revisions that could be hidden while keeping the graph connected.
 
     A second pass will be done to apply "dynamic blocker" like bookmarks or
     working directory parents.