Re: gitk patch collection pull request

From: Jonathan del Strother <maillist@steelskies.com>
Date: Oct 20, 2007, 11:12 UTC
Message-ID: <531A500E-667F-413C-BD20-D23DC817EB72@steelskies.com>
In-Reply-To: <18200.36704.936554.220173@cargo.ozlabs.ibm.com>

On 19 Oct 2007, at 12:05, Paul Mackerras wrote:

Show 24 quoted lines

> Shawn O. Pearce writes:
>
>> The following changes since commit  
>> 719c2b9d926bf2be4879015e3620d27d32f007b6:
>>  Paul Mackerras (1):
>>        gitk: Fix bug causing undefined variable error when cherry- 
>> picking
>>
>> are available in the git repository at:
>>
>>  git://repo.or.cz:/git/spearce.git gitk
>
> OK, but ...
>
>> Jonathan del Strother (2):
>>      gitk: Added support for OS X mouse wheel
>>      Fixing gitk indentation
>
> This one is bogus.  Firstly, it doesn't have "gitk:" at the start of
> the headline (and "Fixing" should be "Fix").  Secondly, the actual
> change itself is bogus.  It changes an initial tab to 8 spaces on each
> of 4 lines.  I like it the way it is - and if he wanted to change it,
> he should have changed it throughout the file, not just on 4 lines.
> So that change is rejected.

In my defense, most of that file is space indented, and the places that are tab indented are generally totally broken unless you have an 8 char tab width. It seems to have the whole 'tabs for code indentation, with space for alignment' rule back-to-front. I can't follow the logic of that, so didn't feel comfortable changing the whole file. I probably shouldn't have submitted the second patch - I initially fixed the weird indentation in my first patch, just so my if- block didn't look totally weird, but then was told that ought to be 2 separate patches.