Re: [PATCH 2/4] Rename remote_only to display_mode

Andy Parkins wrote:
> On Friday 2006 November 03 10:51, Andreas Ericsson wrote:
> 
>> If you *need* to change something, change it. If you *want* to change
>> something just because it's not written the way you would write it, back
>> away. If you think some interface you're using needs clearing up
>> (codewise or with extra comments), send a separate patch for that so the
>> actual feature/bugfix you're sending in doesn't drown in cosmetic
>> changes to the interfaces the patch uses/touches.
> 
> Thank you for the excellent advice.  What then would you suggest in the case 
> in point?  I made as minimal a change as I could make; but that left the code 
> a little bit bitty - I had press-ganged a variable into taking on another 
> function and was using numeric literals that should really have been given 
> meaning with #define?
> 
> My question is perhaps different from simply git-etiquette; it's should I 
> prefer my patches to be minimal or neat?  If there is a more appropriate way 
> of doing something should I do it or should I favour minimalism?
> 

Neat, imo. Re-using old variables might be appropriate if the name of 
the variable still makes sense, but rename it if there's a better name 
for it.

> I've actually rewritten it now as per Junio's request, and while I'm happier 
> with the code, it was much bigger change, that didn't really lend itself to 
> being broken into smaller patches as did my first attempt.
> 
> I guess in the end it's a judgement call and the best thing to do is post it 
> and see who shoots it down :-)
> 

Probably the most sensible approach. Even though the list is pretty 
trigger-happy, the guns are more of the playful water-squirt type than 
the high-powered big-calibre kind.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html