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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]