Thread (2 messages) flat view 2 messages, 2 authors, 2016-08-11

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

From: Junio C Hamano <hidden>
Date: 2016-08-11 19:26:29

Andreas Ericsson [off-list ref] writes:
Andy Parkins wrote:
quoted
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.
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help