Re: [PATCH] Port to 12 other Platforms.

> Boyd Lynn Gerber <gerberb@zenez.com> writes:
> > On Sun, 8 Jun 2008, Jakub Narebski wrote:
> > This was from my own copy of the master archive.  It is my proposal.  I 
> > thought you had to get an OK from this list before you do a push to the 
> > main archive.  Am I missing something?  I am new to this list and the 
> > proper methods for submitting patches.  I thought I was following the 
> > guidelines from 
> >
> > http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD
> >
> > What am I missing?
> 
> It might appear that many people somehow hate your patch and ganging up 
> on it, and if so I apologize for them and I assure you that they do not 
> mean ill.

>    * For an enhancement, describe in what situation the new feature is
>      useful, defend why that use case is worth supporting, state how
>      awkward (or perhaps impossible) to do the same thing is with the
>      current set of features, and discuss and defend why you chose this
>      specific approach to fix the awkwardness among other possibilities.
>      E.g. "This adds a new feature X that works like this.  When you have
>      Y and want to arrive at Z, with the current set of commands you would
>      need to do W, but...".
> 
>    The point is to help people, who later wonder why the change was made
>    and on what basis the author thought the change was necessary and/or
>    sufficient back then when the change was made, understand the context.
> 
>    This comes at the beginning of the e-mail message, and is concluded by
>    S-o-b line(s).

>  * Supporting material that makes it easy to understand the particular
>    iteration of the patch in the context of review discussion, things like
>    "Compared to the previous round, I changed this and that, thanks to
>    comments from X and Y."  Because only the final iteration will get
>    committed in the final history, it does not make sense to include such
>    information in the commit message.  This comes after the commit log
>    message, and a single three-dash line is used to separate this part
>    from the commit log message.
> 
>  * The change itself, aka "patch".  This comes at the end of the message.

>     __USLC__ indicates UNIX System Labs Corperation (USLC), or a Novell-derived
>     compiler and/or some SysV based OS's.
> 
>     __M_UNIX indicates XENIX/SCO UNIX/OpenServer 5.0.7 and prior releases
>     of the SCO OS's.  It is used just like Apple and BSD, both of these
>     shouldn't have _XOPEN_SOURCE defined.
> 
> These are valuable clues to anybody who is unfamiliar with (and/or do 
> not have an easy access to) these systems.  When people later want to 
> touch git-compat-util.h around the place where !defined(__USLC__) is 
> used, they would run "git blame" (or perhaps "git log -S__USLC__") to 
> find your commit that modified this line, and by looking at the commit 
> log message why you added these symbols on the #if line.  It would help 
> protect your changes from begin broken by them if you help them 
> understand why these are there, and the above two paragraphs should 
> definitely go to the commit log message.  They are not mere supporting 
> material for this review cycle alone.

> "..., both of these shouldn't have" however could even be more helpful 
> if it was stated like "On these platforms, defining _XOPEN_SOURCE hides 
> definitions of X, Y and Z that we use, which is not what we want.", for 
> people who would want to know what specific breakage the change 
> addresses.
>
> It would change "Ok, somebody with SCO systems says this patch fixes 
> things for him" to "I see, if _XOPEN_SOURCE over there makes *that* 
> function unavailable, then we definitely shouldn't have _XOPEN_SOURCE 
> defined at this point of the header file".  IOW, it makes "Ok, I trust 
> the guy's judgement, even though the details are fuzzy to me" into "Ok, 
> I agree with his judgement".