From: Andreas Ericsson Date: Fri, 03 Nov 2006 13:23:46 GMT Subject: Re: [PATCH 2/4] Rename remote_only to display_mode Message-ID: <454B42E2.8070001@op5.se> In-Reply-To: <200611031200.27275.andyparkins@gmail.com> 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