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

From: Junio C Hamano <junkio@cox.net>
Date: Nov 3, 2006, 11:51 UTC
Message-ID: <7v4ptgagth.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <454B1F3B.1020603@op5.se>

Andreas Ericsson <ae@op5.se> writes:

Show 13 quoted lines

> Andy Parkins wrote:
>> Digressing a little: what is the polite form of patches for git?  My
>> strategy with this set was to make each patch as small as possible
>> to reach my end point.  If those patches were okayed on the list, I
>> could then do a "make more beautiful" patch, which is really nothing
>> to do with the original changes to functionality but would make the
>> code prettier.
>
> I believe the order of preferrence goes: tested, concise, short.
>
> Linus has a nasty habit of ending his mails with "totally untested
> ofcourse", which is not a good strategy to adopt if you want your
> patches included.

I've picked it up as well. Consider it a privilege for being the toplevel maintainer ;-).

Seriously, it is perfectly Ok to send "for discussion" feelers that are untested or messy, but marking them clearly as "for discussion only -- will clean-up after discussion" would be very much appreciated.

The organization of our four series was almost perfect, except you went a bit too far with [2/4]. I said "taken alone this is regression in readability", with the full knowledge that the real reason of the change was that it would not make any sense to call the variable "remote_only" in [3/4]. IOW, I would have rolled 2 and 3 into a single change.

Show 6 quoted lines

> 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.

This is a very good advice.  I fully agree with Andreas.

When I was an active contributor, somebody (I do not remember who) asked me privately: "you seem to be getting along pretty well with Linus; I have these changes I want to send in, but can you suggest a good strategy to get patches accepted?" I recall saying something along this line:

 - Make sure the patches apply cleanly to upstream (rebase if
   necessary).

 - When making a series, make clean-ups and obviously correct
   ones early, then follow them with bigger and potentially more
   controversial ones.  Doing it the other way around takes the
   "obviously correct" ones hostage to the latter; IOW, you
   would be effectively saying "if you swallow this big change,
   you would get these clean-ups and obviously correct
   bugfixes", which is not nice to the maintainer.

 - When some of your changes are accepted but others were not,
   do not give up, if you believe in the cause, try to come up
   with convincing examples to explain why your change helps
   certain workflows.

 - When doing so, make sure the patches apply cleanly to the
   updated upstream that now has some of your changes.  It might
   have been applied with fix-ups, and other people may have
   made clean-ups in neighbouring area.  You may need to re-roll
   the patch.

 - Linus is not doing this full-time and is a busy person.
   Although "not giving up" is important, do not push him too
   hard.  Try to guess how busy he is and what area his
   attention currently is.  The latter is important for two
   reasons.  (1) If you have to touch the same part of code but
   for a different reason, that would add more work on him.  It
   is better wait until the upstream code settles in the part of
   the code and base your patch on that.  (2) If your change
   needs a deep thinking to swallow, it is likely to be dropped
   when Linus's attention is in completely different area.