Re: [PATCH v2] pull: handle --log=<n>

From: Stefan Beller <sbeller@google.com>
Date: May 19, 2015, 21:33 UTC
Message-ID: <CAGZ79kaw_6Cq5SaLvJHrXDm9bh3+ovAddv9CQJ78m65MYF=SqA@mail.gmail.com>
In-Reply-To: <1432070690.14498.4.camel@kaarsemaker.net>

On Tue, May 19, 2015 at 2:24 PM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:

Show 22 quoted lines

> On di, 2015-05-19 at 06:57 -0700, Junio C Hamano wrote:
>> On Tue, May 19, 2015 at 6:35 AM, Johannes Schindelin
>> <johannes.schindelin@gmx.de> wrote:
>> >
>> > On 2015-05-18 20:18, Junio C Hamano wrote:
>> >>
>> >> So I dunno.  I really wish test_commit didn't create tags and either
>> >> left the tagging to the calling script.
>> >
>> > Or maybe just add a --tag flag to `test_commit` and use that in all cases where the tags were actually needed. Yeah, I think I like that option best.
>>
>> Thanks for inferring what I wanted to say but left unsaid due to my
>> stupidity and
>> lack of proofreading. I meant to follow "either left to the caller"
>> with "or with an
>> option" ;-)
>
> I took a stab at this, adding a --tag option to test_commit and adding
> the option to the test_commit calls that need it (or removing tests'
> reliance on these tags where appropriate, or removing tests' workarounds
> for dealing with these tags when they don't want them), and the result
> is 59 files changed, 280 insertions(+), 281 deletions(-)

I guess most of the line changes are just adding the --tag? And I'd guess (281-280) that there is no huge code inside of test_commit either, so I'd assume it doesn't add clutter, but rather cleans up....

Show 5 quoted lines

>
> A test run on master with GIT_TEST_LONG set causes 1138 calls to
> test_commit on my system, of which 255 now use the --tag option
> (measured with a really crude hack that INCR's some keys in redis at
> appropriate points in test_commit).

... 255 out of 1138 is awesome IMHO! Do you see an improvement in time as well (as in "time make test" is X% faster overall) ?

>
> Is this interesting enough to turn into a proper patch series?

I'd think this is worth making a real patch, definitely!

Thanks, Stefan