Re: Enabling scissors by default?

2013-01-09 Thread Jeff King
On Tue, Jan 08, 2013 at 03:36:09PM -0800, Junio C Hamano wrote:

 You could introduce a new configuration variable am.scissors and
 personally turn it on, though.  Setting that variable *does* count
 as the user explicitly asking for it.

I think we have mailinfo.scissors already.

  I often see patches being tweaked in response to feedback and
  resubmitted, usually with a description of what has changed since the
  previous version.  Such descriptions don't need to be in the change
  log when it is finally applied and seem a perfect use of scissors.
 
 Putting such small logs under --- line is the accepted practice.

Maybe it is just me, but I find the scissors form more readable, because
the cover letter material often serves to introduce and give context
to the patch (e.g., Thanks for your feedback. I've tried to do X, and
it came out well. Here's the patch. serves as an introduction, and
logically comes before the commit message itself).

That does not say anything one way or another about how dangerous or not
it might be to enable scissors by default. Just my two cents that I like
the scissors style for patches that come as part of a discussion (and I
prefer the --- style when making comments on the contents of a patch;
i.e., when the comments make more sense to be read after reading the
commit message to understand what the patch does).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Enabling scissors by default?

2013-01-09 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Jan 08, 2013 at 03:36:09PM -0800, Junio C Hamano wrote:

 You could introduce a new configuration variable am.scissors and
 personally turn it on, though.  Setting that variable *does* count
 as the user explicitly asking for it.

 I think we have mailinfo.scissors already.

Oh, spoiler.  I was hoping that I could entice new people into doing
a little digging on their own X-.


  I often see patches being tweaked in response to feedback and
  resubmitted, usually with a description of what has changed since the
  previous version.  Such descriptions don't need to be in the change
  log when it is finally applied and seem a perfect use of scissors.
 
 Putting such small logs under --- line is the accepted practice.

 Maybe it is just me, but I find the scissors form more readable, because
 the cover letter material often serves to introduce and give context
 to the patch (e.g., Thanks for your feedback. I've tried to do X, and
 it came out well. Here's the patch. serves as an introduction, and
 logically comes before the commit message itself).

 That does not say anything one way or another about how dangerous or not
 it might be to enable scissors by default. Just my two cents that I like
 the scissors style for patches that come as part of a discussion (and I
 prefer the --- style when making comments on the contents of a patch;
 i.e., when the comments make more sense to be read after reading the
 commit message to understand what the patch does).

Yes, scissors have their uses, namely when presenting a patch in a
discussion context.  Otherwise we wouldn't have introduced it in the
first place.

But the desription of what has changed since the previous version
use case I was responding to is where the space below --- is meant
to be used from very early days of Git (the convention established
on the kernel list).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Enabling scissors by default?

2013-01-08 Thread Phillip Susi
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

I was wondering why am's scissors option is not enabled by default.
It seems a very handy feature, but I'm reluctant to use it when
sending patches because the recipient has to notice the scissors and
remember to pass --scissors to git am.

Could this be made the default?

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJQ7JLGAAoJEJrBOlT6nu75iDYIANFiiH50RlL9WKEfaoybeA5K
ZLodBze1TcAYIx2/ad6qY+XCoq98+nVXTkv2IAleDiNlfeIhKD24UTWNCysT8p1J
5KeFfR4paxLJLJKkmSL5s3DJbyjLlJWcxD7vGku6F4k35NmY3VYR4rJ/CVv0YRrs
p4nNG/EXWBo3/ngiL9QS4E65N0CfcOOjn48RQUmk1DGXSFNHP4L1KuJ4dA9cs9BC
5KmNwh5X6OOal0Lf+ezbxzvoGMwQmhBAxx3t8JQR3E22dLQlUq7stlPl5LDd+Cis
XWfNk3B4NuFTum9LqWnM5TN89WCCFh4/pskdRd5ONF51G0jbuF/hBFbwU05qL/4=
=Qd94
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Enabling scissors by default?

2013-01-08 Thread Junio C Hamano
Phillip Susi ps...@ubuntu.com writes:

 I was wondering why am's scissors option is not enabled by default.

It is very easy to miss misidentification of scissors line; as a
dangerous, potentially information losing option, I do not think it
should be on by default.

Another reason (and this is the original one) why it is not enabled
is to discourage the contributors from overusing scissors -- 8 --
line.  If you always have to write too much stuff before the proper
explanation of your patch, so that the integrator has to use -c
option all the time, you are explaining your patches wrong.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Enabling scissors by default?

2013-01-08 Thread Phillip Susi
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/08/2013 05:42 PM, Junio C Hamano wrote:
 It is very easy to miss misidentification of scissors line; as a 
 dangerous, potentially information losing option, I do not think
 it should be on by default.

I suppose if it only requires one instance of 8 or 8 and one -, it
might be *slightly* dangerous, but if it required a slightly longer
minimum line length, it would be pretty darn unlikely to get triggered
by accident, and of course, is easily disabled.

 Another reason (and this is the original one) why it is not
 enabled is to discourage the contributors from overusing scissors
 -- 8 -- line.  If you always have to write too much stuff before
 the proper explanation of your patch, so that the integrator has to
 use -c option all the time, you are explaining your patches wrong.

I often see patches being tweaked in response to feedback and
resubmitted, usually with a description of what has changed since the
previous version.  Such descriptions don't need to be in the change
log when it is finally applied and seem a perfect use of scissors.

Usually such version to version descriptions are put in a cover
letter, but if you are only submitting a single patch instead of an
entire series, using a cover letter seems silly when you could just
put the comments in one email and clearly mark them as not needing to
go into the final changelog.


-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJQ7KriAAoJEJrBOlT6nu755UkIALIT3T5yHH5i+0HOrXLlXzQR
+S2jJfFZ8Kcc+kleiEJ3uLFVGTLMpRyjJFKceOuB4/TdJFUivrYJHWJxcKmW8WzK
BJKZOjt/jv9r8Qt/AB7KA45S7awfQnOWkg6KQlJa1IM0nUPbo4upgMlWar9l7vjz
Hkr7geuHY4fsVUJ7R0rYPcT3pue8ywsT4a9o/ocstfXmC05IrLKQtzO4TuvfiaTb
yBG+rAPKz36zfxCN5NyKExZO6v/LnCKym/PH4a6wYIeTUz1EvuaPy5lQOo6ORQ4h
xbSyBRDPN4yiVgNXfSQmGKwd9XPqs6h8Z0q3X5mGZyOXurw0JFRJlJ3v8hHIvqg=
=Rn7z
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Enabling scissors by default?

2013-01-08 Thread Junio C Hamano
Phillip Susi ps...@ubuntu.com writes:

 On 01/08/2013 05:42 PM, Junio C Hamano wrote:
 It is very easy to miss misidentification of scissors line; as a 
 dangerous, potentially information losing option, I do not think
 it should be on by default.

 I suppose if it only requires one instance of 8 or 8 and one -, it
 might be *slightly* dangerous, but if it required a slightly longer
 minimum line length, it would be pretty darn unlikely to get triggered
 by accident, and of course, is easily disabled.

Easily disabled is never a good enough reason to change the long
established default of not doing anything funky unless the user
explicitly asks it to do things differently.

You could introduce a new configuration variable am.scissors and
personally turn it on, though.  Setting that variable *does* count
as the user explicitly asking for it.

 I often see patches being tweaked in response to feedback and
 resubmitted, usually with a description of what has changed since the
 previous version.  Such descriptions don't need to be in the change
 log when it is finally applied and seem a perfect use of scissors.

Putting such small logs under --- line is the accepted practice.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html