Patchwork [01,of,10,V3] discovery: minor fix to some conditionals

login
register
mail settings
Submitter Boris Feld
Date Jan. 4, 2019, 10:45 p.m.
Message ID <562198ca3bb37b9764df.1546641920@localhost.localdomain>
Download mbox | patch
Permalink /patch/37469/
State Accepted
Headers show

Comments

Boris Feld - Jan. 4, 2019, 10:45 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1546620599 -3600
#      Fri Jan 04 17:49:59 2019 +0100
# Node ID 562198ca3bb37b9764dfe93d56cbbf70c2bb093d
# Parent  5c68b617ba2463eb6f1372a24b139a376c6bf6bd
# EXP-Topic discovery-refactor
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 562198ca3bb3
discovery: minor fix to some conditionals

Since `size` is the upper limit of the sample, we should include it in the
check. Otherwize the `more` variable will be zero and the sampling will be
useless
Pulkit Goyal - Jan. 5, 2019, 8:57 a.m.
On Sat, Jan 5, 2019 at 4:15 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1546620599 -3600
> #      Fri Jan 04 17:49:59 2019 +0100
> # Node ID 562198ca3bb37b9764dfe93d56cbbf70c2bb093d
> # Parent  5c68b617ba2463eb6f1372a24b139a376c6bf6bd
> # EXP-Topic discovery-refactor
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 562198ca3bb3
> discovery: minor fix to some conditionals
>
> Since `size` is the upper limit of the sample, we should include it in the
> check. Otherwize the `more` variable will be zero and the sampling will be
> useless
>

Queued 1-2 for now. Many thanks!
Yuya Nishihara - Jan. 6, 2019, 2:04 a.m.
On Fri, 04 Jan 2019 23:45:20 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1546620599 -3600
> #      Fri Jan 04 17:49:59 2019 +0100
> # Node ID 562198ca3bb37b9764dfe93d56cbbf70c2bb093d
> # Parent  5c68b617ba2463eb6f1372a24b139a376c6bf6bd
> # EXP-Topic discovery-refactor
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 562198ca3bb3
> discovery: minor fix to some conditionals
> 
> Since `size` is the upper limit of the sample, we should include it in the
> check. Otherwize the `more` variable will be zero and the sampling will be
> useless
> 
> diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
> --- a/mercurial/setdiscovery.py
> +++ b/mercurial/setdiscovery.py
> @@ -146,7 +146,7 @@ def _takefullsample(repo, headrevs, revs
>      _updatesample(revs, revsroots, sample, children.__getitem__)
>      assert sample
>      sample = _limitsample(sample, size)
> -    if len(sample) < size:
> +    if len(sample) <= size:
>          more = size - len(sample)
>          sample.update(random.sample(list(revs - sample), more))

The original condition looks more correct since there's no reason to pick
0 more revision if len(sample) == size.
Boris Feld - Jan. 10, 2019, 8:56 a.m.
On 06/01/2019 03:04, Yuya Nishihara wrote:
> On Fri, 04 Jan 2019 23:45:20 +0100, Boris Feld wrote:
>> # HG changeset patch
>> # User Boris Feld <boris.feld@octobus.net>
>> # Date 1546620599 -3600
>> #      Fri Jan 04 17:49:59 2019 +0100
>> # Node ID 562198ca3bb37b9764dfe93d56cbbf70c2bb093d
>> # Parent  5c68b617ba2463eb6f1372a24b139a376c6bf6bd
>> # EXP-Topic discovery-refactor
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 562198ca3bb3
>> discovery: minor fix to some conditionals
>>
>> Since `size` is the upper limit of the sample, we should include it in the
>> check. Otherwize the `more` variable will be zero and the sampling will be
>> useless
>>
>> diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
>> --- a/mercurial/setdiscovery.py
>> +++ b/mercurial/setdiscovery.py
>> @@ -146,7 +146,7 @@ def _takefullsample(repo, headrevs, revs
>>      _updatesample(revs, revsroots, sample, children.__getitem__)
>>      assert sample
>>      sample = _limitsample(sample, size)
>> -    if len(sample) < size:
>> +    if len(sample) <= size:
>>          more = size - len(sample)
>>          sample.update(random.sample(list(revs - sample), more))
> The original condition looks more correct since there's no reason to pick
> 0 more revision if len(sample) == size.

Indeed, sending a followup. What happened to changeset 5+ in this
series? I see them neither queued nor with change requests.

Cheers,

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

Patch

diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
--- a/mercurial/setdiscovery.py
+++ b/mercurial/setdiscovery.py
@@ -146,7 +146,7 @@  def _takefullsample(repo, headrevs, revs
     _updatesample(revs, revsroots, sample, children.__getitem__)
     assert sample
     sample = _limitsample(sample, size)
-    if len(sample) < size:
+    if len(sample) <= size:
         more = size - len(sample)
         sample.update(random.sample(list(revs - sample), more))
     return sample
@@ -264,7 +264,7 @@  def findcommonheads(ui, local, remote,
             ui.debug("taking quick initial sample\n")
             samplefunc = _takequicksample
             targetsize = initialsamplesize
-        if len(undecided) < targetsize:
+        if len(undecided) <= targetsize:
             sample = list(undecided)
         else:
             sample = samplefunc(local, ownheads, undecided, targetsize)