All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luke Diamand <luke@diamand.org>
To: Lex Spoon <lex@lexspoon.org>
Cc: Git Users <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCHv1 3/3] git-p4: fixing --changes-block-size handling
Date: Sun, 07 Jun 2015 18:06:13 +0100	[thread overview]
Message-ID: <55747A05.3070704@diamand.org> (raw)
In-Reply-To: <CALM2SnbVY1baAONo3o2gb2NS+rDSsyhkPffP5EJZKU1MDA7q9w@mail.gmail.com>

On 07/06/15 17:33, Lex Spoon wrote:
> The implementation looks fine, especially given the test cases that
> back it up. I am only curious why the block size is set to a default
> of None. To put it as contcretely as possible: is there any expected
> configuration where None would work but 500 would not? We know there
> are many cases of the other way around, and those cases are going to
> send users to StackOverflow to find the right workaround.

I think it was just caution: it's pretty easy to make it fall back to 
the old non-batched scheme, so if it turns out that there *is* a 
problem, fewer people will hit the problem and we're less likely to have 
a paper-bag release.

>
> Dropping the option would also simplify the code in several places.
> The complex logic around get_another_block could be removed, and
> instead there could be a loop from start to mostRecentCommit by
> block_size. Several places that check "if not block_size" could just
> choose the other branch.

Fair point. I'll give it a go and see what happens.

(Plus 500 is a very unnatural number, chosen just because we still place 
some kind of significance on a chance evolutionary accident that gave 
our ape ancestors 5 digits on each hand :-)

Luke

  reply	other threads:[~2015-06-07 17:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-07 10:21 [PATCHv1 0/3] git-p4: fixing --changes-block-size support Luke Diamand
2015-06-07 10:21 ` [PATCHv1 1/3] git-p4: additional testing of --changes-block-size Luke Diamand
2015-06-07 16:06   ` Lex Spoon
2015-06-07 10:21 ` [PATCHv1 2/3] git-p4: test with limited p4 server results Luke Diamand
2015-06-07 16:11   ` Lex Spoon
2015-06-07 10:21 ` [PATCHv1 3/3] git-p4: fixing --changes-block-size handling Luke Diamand
2015-06-07 16:33   ` Lex Spoon
2015-06-07 17:06     ` Luke Diamand [this message]
2015-06-07 21:35       ` [PATCHv2 0/3] " Luke Diamand
2015-06-07 21:35         ` [PATCHv2 1/3] git-p4: additional testing of --changes-block-size Luke Diamand
2015-06-07 21:35         ` [PATCHv2 2/3] git-p4: test with limited p4 server results Luke Diamand
2015-06-07 21:35         ` [PATCHv2 3/3] git-p4: fixing --changes-block-size handling Luke Diamand
2015-06-07 22:58           ` Lex Spoon
2015-06-08 16:02             ` Junio C Hamano
2015-06-08 16:36               ` Lex Spoon
     [not found]                 ` <xmqqy4juazkz.fsf@gitster.dls.corp.google.com>
     [not found]                   ` <5575E264.6040601@diamand.org>
2015-06-08 22:32                     ` Junio C Hamano
2015-06-07 16:01 ` [PATCHv1 0/3] git-p4: fixing --changes-block-size support Lex Spoon
2015-06-07 16:58   ` Luke Diamand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55747A05.3070704@diamand.org \
    --to=luke@diamand.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=lex@lexspoon.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.