Re: [PATCH 1/5] p5550: factor our nonsense-pack creation

[Jeff King]

On Thu, Nov 23, 2017 at 11:41:25AM +0900, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Right, I came to the same conclusion (we may even have discussed this on
> > the list, I don't remember). The current "todo" format says that only
> > the command and sha1 matter, and we'd be changing that. Maybe that's not
> > so bad if the user has to enable the feature themselves (and clearly it
> > would be incompatible with a custom format option).
> 
> If you are in the habit of always writing 4 or more lines, the
> chance that you would make a typo on the line you can correct in the
> "todo" list with such a feature is at most 25% ;-).

You'd think, but the distribution of typos doesn't seem to be uniform.
:)

I think what actually happens is that I quite often do my final
proofread in my MUA, where the subject is displayed apart from the rest
of the message. So I tend to gloss over it, and any errors there are
more likely to make it to the list.

> I think one downside is that a mere presence of such an option hints
> that we somehow encourage people to commit with a title-only message.

Yeah, though it might be handy for me I think it just opens up too many
funny questions like this.

-Peff

Re: [PATCH 1/5] p5550: factor our nonsense-pack creation

[Junio C Hamano]

Jeff King  writes:

> Right, I came to the same conclusion (we may even have discussed this on
> the list, I don't remember). The current "todo" format says that only
> the command and sha1 matter, and we'd be changing that. Maybe that's not
> so bad if the user has to enable the feature themselves (and clearly it
> would be incompatible with a custom format option).

If you are in the habit of always writing 4 or more lines, the
chance that you would make a typo on the line you can correct in the
"todo" list with such a feature is at most 25% ;-).

I think one downside is that a mere presence of such an option hints
that we somehow encourage people to commit with a title-only message.

Re: [PATCH 1/5] p5550: factor our nonsense-pack creation

[Jeff King]

On Tue, Nov 21, 2017 at 04:32:42PM -0800, Stefan Beller wrote:

> > Heh, yes. I even fixed it once, but I have the funny habit of noticing
> > such typos while reading the "todo" list of "rebase -i" and fixing them
> > there. Which of course has no impact whatsoever on the commit. :-/
> 
> That happened to me a couple of times as well before.
> This sounds like a UX bug on first thought.
> 
> On second thought the text after the abbreviated hash can be
> user dependent IIRC, by setting some format option how to populate the
> rebase instruction sheet using log, so it would not be easy to take
> fixes from that line into the commit for a fixup.

Right, I came to the same conclusion (we may even have discussed this on
the list, I don't remember). The current "todo" format says that only
the command and sha1 matter, and we'd be changing that. Maybe that's not
so bad if the user has to enable the feature themselves (and clearly it
would be incompatible with a custom format option). But I think in the
end it probably just makes sense to retrain my expectation, and remember
to "reword" instead of "pick".

-Peff

Re: [PATCH 1/5] p5550: factor our nonsense-pack creation

[Stefan Beller]

On Tue, Nov 21, 2017 at 7:58 AM, Jeff King  wrote:
> On Mon, Nov 20, 2017 at 06:55:51PM -0500, Eric Sunshine wrote:
>
>> On Mon, Nov 20, 2017 at 3:26 PM, Jeff King  wrote:
>> > p5550: factor our nonsense-pack creation
>>
>> s/our/out/, I guess.
>
> Heh, yes. I even fixed it once, but I have the funny habit of noticing
> such typos while reading the "todo" list of "rebase -i" and fixing them
> there. Which of course has no impact whatsoever on the commit. :-/
>

That happened to me a couple of times as well before.
This sounds like a UX bug on first thought.

On second thought the text after the abbreviated hash can be
user dependent IIRC, by setting some format option how to populate the
rebase instruction sheet using log, so it would not be easy to take
fixes from that line into the commit for a fixup.

Re: [PATCH 1/5] p5550: factor our nonsense-pack creation

[Jeff King]

On Mon, Nov 20, 2017 at 06:55:51PM -0500, Eric Sunshine wrote:

> On Mon, Nov 20, 2017 at 3:26 PM, Jeff King  wrote:
> > p5550: factor our nonsense-pack creation
> 
> s/our/out/, I guess.

Heh, yes. I even fixed it once, but I have the funny habit of noticing
such typos while reading the "todo" list of "rebase -i" and fixing them
there. Which of course has no impact whatsoever on the commit. :-/

-Peff

Re: [PATCH 1/5] p5550: factor our nonsense-pack creation

[Eric Sunshine]

On Mon, Nov 20, 2017 at 3:26 PM, Jeff King  wrote:
> p5550: factor our nonsense-pack creation

s/our/out/, I guess.

> We have a function to create a bunch of irrelevant packs to
> measure the expense of reprepare_packed_git(). Let's make
> that available to other perf scripts.
>
> Signed-off-by: Jeff King 

[PATCH 1/5] p5550: factor our nonsense-pack creation

[Jeff King]

We have a function to create a bunch of irrelevant packs to
measure the expense of reprepare_packed_git(). Let's make
that available to other perf scripts.

Signed-off-by: Jeff King 
---
 t/perf/lib-pack.sh | 29 +
 t/perf/p5550-fetch-tags.sh | 25 ++---
 2 files changed, 31 insertions(+), 23 deletions(-)
 create mode 100644 t/perf/lib-pack.sh

diff --git a/t/perf/lib-pack.sh b/t/perf/lib-pack.sh
new file mode 100644
index 00..501bb7b272
--- /dev/null
+++ b/t/perf/lib-pack.sh
@@ -0,0 +1,29 @@
+# Helpers for dealing with large numbers of packs.
+
+# create $1 nonsense packs, each with a single blob
+create_packs () {
+   perl -le '
+   my ($n) = @ARGV;
+   for (1..$n) {
+   print "blob";
+   print "data <