On Sun, 8 Jun 2008, Junio C Hamano wrote: > Boyd Lynn Gerber <gerberb@xxxxxxxxx> 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. This list has been very good. The problem comes from other lists and personal assualts on my domain. > There seem to be some confusion either in the SubmittingPatches document > or the way some suggestions have been given in the recent postings by > people, so let's clear it up first. Yes, I was a bit confused but the docs/email/IRC. I really apperciate the message below. I really want to comply with the rules of this list and make sure my changes make it into the master/core source. ... > * 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). I agree. I am not sure on some things but I will ask more later. > * 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. I will have to find all this information. It took me 2 months in my personal time to find and fix them. I will have to get back on this below. > "..., 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". Thanks, more later when time permits. -- Boyd Gerber <gerberb@xxxxxxxxx> ZENEZ 1042 East Fort Union #135, Midvale Utah 84047 -- 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
