Re: [PATCH 1/5] Fixing Lesser GPL version number [1/5]

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5207 bytes --]

On 10/9/20 1:37 AM, Chetan Pant wrote:

Thank you for contributing.  However, it will require more work before
this is ready to merge; while I have a lot of comments below, I hope you
read them in the intended tone of ideas on making your v2 better.  For
more thoughts on patch submission, see
https://wiki.qemu.org/Contribute/SubmitAPatch

Your message was not threaded with the other 4 patches, nor did you put
them in-reply-to a 0/5 cover letter.  This makes it harder to keep the
patches together in mail clients that sort by thread activity.

> There is no "version 2" of the "Lesser" General Public License. It is
> either "GPL version 2.0" or "Lesser GPL version 2.1". This patch replaces all
> occurrences of "Lesser GPL version 2" with "Lesser GPL version 2.1" in comment section.

I'm not a copyright lawyer; there may be legal ramifications for doing
this on files where you are not the copyright owner, although to me it
looks like an obvious correction of intent, so I'm okay with the idea.

Your subject line is awkward, for several reasons.  First, it does not
use imperative tense.  A handy rule of thumb: try mentally prepending
the phrase "apply this patch in order to..." in front of your git
commit; where the implied lead-in tends to make sense only if the
explicit text used imperative tense.  So in this case, you would do
s/Fixing/Fix/.  More comments below [1]

> Also, It came to notice that some of the files that were edited for the change

s/It/I/

> were not following latest comment rules. For example using "//" to mark comment
> instead of "/*". That is also fixed in this patch.

When writing a commit message that starts with "Also," that is a strong
indication that you are fixing two distinct problems in one commit.
Don't fall for that temptation - logically separate changes should be
separate commits, as it makes review easier.  I had to go check in this
patch which file had the // comment change, but based on the diffstat
alone, none of them did, which makes it feel like this is a stale
copy-and-paste from one of your other commits in the series that merged
too much work into one commit.

> 
> This patch is divided in 5 parts, directory wise, in order to make reviewing process easier.
> Below listed are the parts of the patch, where asterisk denotes the part you are currently viewing.
> 
> 	[*] Files in authz/backends/block/linux-user/tests/migration directory (82 Files)
> 	[ ] Files in hw/include/disas (100 files)
> 	[ ] Files inside target/ 'alpha,arm,cris,hppa,i386' (96 files)
> 	[ ] Files inside target/ 'lm32,microblaze,mips,ppc,rx,sparc,tilegx,tricore,xtensa' (63 files)
> 	[ ] Files in ui/util/include/scripts and QEMU root directory (76 Files)

[1]Back to the topic of the commit subject.  A quick search of qemu.git
history shows that using a [1/5] designation is invention:

$ git shortlog | grep '\[[0-9]*/[0-9]*'
      tests/fp: enable f128_to_ui[32/64] tests in float-to-uint
      fpu: convert float[16/32/64]_squash_denormal to new modern style

But if you drop the [1/5] designation, then you would be submitting five
distinct patches with identical subjects, which is a backport nightmare.
 Better is to leave the patch ordering information in JUST the initial
[PATCH n/m] prefix (since that prefix is auto-stripped by git am), and
instead differentiate your subject lines with a topic.

Thus, maybe something like:

backends: Fix LGPL version number
disas: Fix LGPL version number
target: Fix LGPL version number

and so on.  There may be a smarter division of the work based on which
maintainer(s) will have to ack various patches; breaking the series into
more than 5 chunks may be easier, although it then requires more topic
lines.  Or, if it truly is automated, doing it all in one shot may be
best after all.

> 
> Below is how the license version was corrected:
> 
> 1. To find the number of file having "Lesser GPL version 2 ":
> 	grep -l Lesser $(grep -rl "version 2 " * ) > result.dat
>    Total of 417 files were found (After manually exluding the files like COPYING and COPYING.LIB from the result)

excluding

Long line; try to keep commit message bodies wrapped at column 72 or
less (since 'git log' displays them with indentation, and it is still
common to read git log in an 80-column window)

> 
> 2. To find the number of occurences of "version 2 " in the resulted files:

occurrances

> 	egrep -c "version 2 " $(cat result.dat)
>    410 files had "version 2" occurence 1 time (name of those files was saved in one_timers.dat)
>    and in 7 files "version 2" occurences were multiple times.
> 
> 3. Files having occurence exactly 1 time were corrected using below command:

three more of the same typo

> 	sed -i "s/version 2 /version 2.1 /g" $(cat one_timers.dat)
>    For rest of 7 files, correction was done manually.
> 
> Signed-off-by: Chetan Pant <chetan4windows@gmail.com>
> ---

I did not actually inspect the generated changes to see if they all look
reasonable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]