Re: [PATCH 2/2] gitlog-to-changelog: support 'tiny change' commits.

[Gary V. Vaughan]

Thanks Karl and Jim for the feedback, resubmitting for approval below.

On 10 Nov 2011, at 23:47, Karl Berry wrote:
>> On 1 Nov 2011, at 20:13, Jim Meyering wrote:

>>> Actually, I question whether establishing such a convention.

>>> and going to this trouble is worthwhile, since the size of the
>>> change is already known, via the associated patch.
>> 
>> Gary V. Vaughan wrote:
>> This is a confusion brought about by the 'Tiny change' misnomer I think.  If
>> it were a simple matter of counting lines, then you would be entirely correct
>> of course, but what about a refactoring that moves code around considerably
>> and generates a huge patch, potentially with a large difference between 
>> number
>> of lines added vs number of lines removed, even though no new code is added?
>> That would certainly qualify for 'Copyright-paperwork-required: No'/'tiny 
>> change'.

>    Can we just take the difference between lines added and lines
>    removed per patch, and automatically add the (tiny change) annotation
>    to the generated ChangeLog if that turns out to be 5 or less?  I
>    tend to think not, 
> 
> I tend to agree with you, for the reasons you state.  (Unfortunately.)
> 
>    I'm also interested in whether you have an opinion on my preference
>    for 'Copyright-paperwork-required' as the VCS tag, 
> 
> Sounds good to me, again for the reasons you state.

Hence, I'm resubmitting this patch mostly unchanged, but rebased against master.

> On 1 Nov 2011, at 20:13, Jim Meyering wrote:
>> Gary V. Vaughan wrote:
>>> 
>>> diff --git a/ChangeLog b/ChangeLog
>>> index f370be6..d59d9f9 100644
>>> --- a/ChangeLog
>>> +++ b/ChangeLog
>>> @@ -1,5 +1,14 @@
>>> 2011-11-01  Gary V. Vaughan  <address@hidden>
>>> 
>>> +   gitlog-to-changelog: support `tiny change' commits.
>>> +   The FSF insist that all non-trivial patches to its projects are
>> 
>> Even if somewhere you've felt "insistence" on this issue,
>> let's just write "request":
>> 
>> s/insist/request/

I've used require, which is not quite as strong as insist, but truer than
request.

>> s/are/be/ (both above and below)

Done.

>>> +   accompanied by appropriate paperwork, or that any patches that are
>>> +   applied without that paperwork are marked as such in the
>>> +   ChangeLog.
>>> +   * gitlog-to-changelog: Convert `Copyright-paperwork-required: No'
>> 
>> I find that rather long, and have a slight preference
>> to avoid an always-negative setting.
> 
>> Did you consider "Tiny-change: Yes"
>> or even simply "/^Tiny[- ]change\b/",
> 
> I did consider, but the whole Tiny Change terminology seems anachronistic to
> me.  Although we have long settled on the unfortunate convention of annotating
> ChangeLog entries with "(tiny change)" whenever a patch is considered by the
> committer to be trivial enough not to require an exchange of copyright 
> paperwork,
> I think it is clearer and more sensible to state that directly than be forever
> explaining to patch submitters that we're not belittling their contribution, 
> but
> merely noting that we don't require them to file a copyright disclaimer.
> 
> I chose the negative version to optimise for frequency.  I would hate to have
> to add 'Copyright-paperwork-on-file: Yes' to almost every patch.  I guess I'd
> be happy with 'Copyright-paperwork-not-required: .*$', if it's just the 'No'
> that you dislike... although it feels a lot more awkward to write
> 'xxx-not-required: Yes' than a simple 'xxx-required: No'.

I think 'Copyright-paper-required: No' is still the best compromise here for
the reasons stated earlier in the thread.

Okay to push?


The FSF require that all non-trivial patches to its projects be
accompanied by appropriate paperwork, or that any patches that are
applied without that paperwork are marked as such in the
ChangeLog.
* gitlog-to-changelog: Convert `Copyright-paperwork-required: No'
lines from the git log message to standard `(tiny change)'
ChangeLog annotation.
* scripts/git-hooks/commit-msg: Diagnose redundant or malformed
Copyright-paperwork-required lines.
---
 ChangeLog                     |   13 +++++++++++++
 build-aux/gitlog-to-changelog |    8 +++++++-
 scripts/git-hooks/commit-msg  |   28 +++++++++++++++++++++++++++-
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9b0fa82..7799f7a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2011-11-01  Gary V. Vaughan  <address@hidden>
 
+       gitlog-to-changelog: support `tiny change' commits.
+       The FSF requires that all non-trivial patches to its projects be
+       accompanied by appropriate paperwork, or that any patches that are
+       applied without that paperwork are marked as such in the
+       ChangeLog.
+       * gitlog-to-changelog: Convert `Copyright-paperwork-required: No'
+       lines from the git log message to standard `(tiny change)'
+       ChangeLog annotation.
+       * scripts/git-hooks/commit-msg: Diagnose redundant or malformed
+       Copyright-paperwork-required lines.
+
+2011-11-01  Gary V. Vaughan  <address@hidden>
+
        gitlog-to-changelog: support multi-author commits.
        The FSF cares about keeping track of all authors of patches to its
        projects, but Git doesn't provide obvious support for multi-author
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index 9d98b82..0b3a544 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -251,6 +251,11 @@ sub parse_amend_file($)
 
       my $date_line = sprintf "%s  $2\n", strftime ("%F", localtime ($1));
 
+      # Format 'Copyright-paperwork-required: No' as a standard ChangeLog
+      # `(tiny change)' annotation.
+      my $tiny_change = grep /^Copyright-paperwork-required:\s+[Nn]o$/, @line;
+      $date_line =~ s/$/  (tiny change)/ if $tiny_change;
+
       # Format 'Co-authored-by: A U Thor <address@hidden>' lines in
       # standard multi-author ChangeLog format.
       my @coauthors = grep /^Co-authored-by:.*$/, @line;
@@ -277,9 +282,10 @@ sub parse_amend_file($)
       $prev_date_line = $date_line;
       @prev_coauthors = @coauthors;
 
-      # Omit "Co-authored-by..." and "Signed-off-by..." lines.
+      # Omit meta-data lines we've already interpreted.
       @line = grep !/^Signed-off-by: .*>$/, @line;
       @line = grep !/^Co-authored-by: /, @line;
+      @line = grep !/^Copyright-paperwork-required: /, @line;
 
       # Remove leading and trailing blank lines.
       if (@line)
diff --git a/scripts/git-hooks/commit-msg b/scripts/git-hooks/commit-msg
index 5efdf1f..3d9bfef 100755
--- a/scripts/git-hooks/commit-msg
+++ b/scripts/git-hooks/commit-msg
@@ -1,6 +1,7 @@
 #!/bin/sh
 # An example hook script for catching duplicate or malformed
-# Co-authored-by lines in the commit message.
+# Co-authored-by or Copyright-paperwork-required lines in the
+# commit message.
 
 : ${SED="sed"}
 test "${ECHO+set}" = set || ECHO='printf %s\n'
@@ -53,6 +54,7 @@ fn_check_msg ()
     return_status=0
 
     CAB_re='^Co-authored-by: '
+    CPR_re='^Copyright-paperwork-required: '
 
     # Flag duplicated Co-authored-by lines.
     dups=`grep "$CAB_re" "$log_file" 2>/dev/null \
@@ -75,6 +77,30 @@ fn_check_msg ()
             }
          done
 
+    # Flag duplicated Copyright-paperwork-required lines.
+    count=`grep "$CPR_re" "$log_file" 2>/dev/null \
+      |wc |sed -e 's,^[         ]*,,;s,[       ].*$,,'`
+
+    test 2 -gt "$count" || {
+      $ECHO 'More than one Copyright-paperwork-required line.'
+      return_status=1
+    }
+
+    # Make sure Copyright-paperwork-required line is valid.
+    if grep "${CPR_re}[Yy]" "$log_file" >/dev/null 2>&1; then
+      $ECHO "\
+\`Copyright-paperwork-required: Yes' is redundant, please remove."
+      return_status=1
+    else
+      not_no=`grep "${CPR_re}" "$log_file" 2>/dev/null \
+        |grep -v "${CPR_re}No\$"`
+
+      test -n "$not_no" && {
+        $ECHO "\`Copyright-paperwork-required' setting must be \`No'."
+        return_status=1
+      }
+    fi
+
     return $return_status
 }
 
-- 
1.7.7.3

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)