Hi Junio, On Wed, Jun 01, 2011 at 22:44:30, Junio C Hamano wrote: > Sekhar Nori <nsekhar@xxxxxx> writes: > > > +am.signoff:: > > + A boolean value which lets you enable the `-s/--signoff` option of > > + am by default. *Note:* Adding the Signed-off-by: line to a patch > > + should be a conscious act and means that you certify you have > > + the rights to submit this work under the same open source license. > > + Please see the 'SubmittingPatches' document for further discussion. > > I see you already have done your homework, which is much better than > previous attempts by other people to add an option to add sign-off to > various commands (commit and format-patch), but there are at least three > problems here: Yes, I guessed there would have been previous attempts at this and actually did search the mail archives a bit before sending the patch. I actually didn't hit anything so looks like I probably used the wrong keywords. > * This is an end-user document, not a document for git developers, so > they are not bound by SubmittingPatches at all. They do not necessarily > have SubmittingPatches document in the first place; I will be honest here, I just took this note from the existing documentation for format.signoff config option. May be I can instead say: "Please check the guidelines on submitting patches for your project for further discussion on this. 'SubmittingPatches' document in Linux kernel or 'git' sources is an example of such a document." > * Once you set this configuration variable, I do not see a way to > countermand "This is not signed-off" from the command line for a single > invocation of "am" in your patch; Okay. I guess adding an --no-signoff to git am should help here? > > * If it should be a "conscious act", it shouldn't be "set once and forget > about it" configuration option. You should be making the conscious > decision for each piece of e-mail if it is in line with the project's > Sign-off policy. > > The last one means that the patch is internally inconsistent. In a project > (not this project) where sign-off is used but it does not require signing > off to be a conscious act, I see it may make sense to make it easier to > sign-off a patch when generating (i.e. "commit -s"), preparing to send > (i.e. "format-patch -s"), or accepting (i.e. "am -s"). And this option > might be a possible way to do so. But in that case, the option description > wouldn't say "it should be a conscious act". > > Also if it doesn't have to be a conscious act, what value does such a > boilerplate have? Such a project may be better off not using sign-off at > all to begin with. Its hard to argue against this. All I would say is it is probably much safer to enable sign off by default when accepting a patch than when preparing to send it. Yet, we have format.signoff but no am.signoff. On any project with conscious sign-off rules, one would never accept a patch without a sign-off from the original developer. So, just making it easier to accept patches which are already signed-off should be okay, I guess? May be we should look for an existing sign-off in the mbox and only then have this option come into play? Thanks, Sekhar -- 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