| 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
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.
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.)
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,
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.
> 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
> 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 >
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.