diff mbox

[0/6] Generate Complex functions from a common template.

Message ID ce06c4c6-22f7-51ee-fdfc-a29c7d7e3a46@redhat.com
State New
Headers show

Commit Message

Pedro Alves July 1, 2016, 9:58 a.m. UTC
I got curious here and used "git diff -B -M30%" to (much) more
clearly see the real differences in the patches that mostly do
conversion to generated code, and spotted this funny change:

similarity index 37%
rename from math/s_clog10f.c
rename to math/b_clog10.c
index 485625e..eeb2d65 100644

:-)

(there are likely others, I stopped looking here.)

(I'd suggest considering posting the next revision of the series
with -M30% or some such threshold that catches all renames, to
ease review, but that's up to Joseph).

Thanks,
Pedro Alves

Comments

Paul E. Murphy July 1, 2016, 3:11 p.m. UTC | #1
On 07/01/2016 04:58 AM, Pedro Alves wrote:
> I got curious here and used "git diff -B -M30%" to (much) more
> clearly see the real differences in the patches that mostly do
> conversion to generated code, and spotted this funny change:
> 
> similarity index 37%
> rename from math/s_clog10f.c
> rename to math/b_clog10.c
> index 485625e..eeb2d65 100644
> --- c/math/s_clog10f.c
> +++ w/math/b_clog10.c
> @@ -6,9 +6,8 @@
>     The GNU C Library is free software; you can redistribute it and/or
>     modify it under the terms of the GNU Lesser General Public
>     License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> +   version M_LIT (2.1) of the License, or (at your option) any later version.
> +The GNU C Library is distributed in the hope that it will be useful,
>     but WITHOUT ANY WARRANTY; without even the implied warranty of
>     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>     Lesser General Public License for more details.
> 
> :-)

Ah, some of the sausage making process spills out.  I've fixed those,
and improved (or fixed) the first line comments on most of the files
too.  Likewise, I fixed an M_LIT (0.0) translation inside another
comment after Joseph's suggestion for patch 3.

> 
> (there are likely others, I stopped looking here.)
> 
> (I'd suggest considering posting the next revision of the series
> with -M30% or some such threshold that catches all renames, to
> ease review, but that's up to Joseph).

I toyed around with various levels and options, none seem to
a good job of handling the renames.  I'm not sure there is a
one size fits all solution to getting a good, concise patch.

A thorough review might involve diffing the b_*.c file with
the equivalent real type file pre-patch.


Thanks,
Paul
Zack Weinberg July 1, 2016, 3:15 p.m. UTC | #2
On Fri, Jul 1, 2016 at 11:11 AM, Paul E. Murphy
<murphyp@linux.vnet.ibm.com> wrote:
>
> I toyed around with various levels and options, none seem to
> a good job of handling the renames.  I'm not sure there is a
> one size fits all solution to getting a good, concise patch.
>
> A thorough review might involve diffing the b_*.c file with
> the equivalent real type file pre-patch.

Is it feasible for you to divide each patch that renames files into
one chunk that _just_ performs renames, and another that makes content
changes?  Obviously they would need to be squashed back together on
landing, otherwise the tree would be broken in the
renamed-but-not-edited changeset, but it would make it much easier to
review.  And I realize this might be a huge amount of tedious manual
work.

zw
Paul E. Murphy July 1, 2016, 3:47 p.m. UTC | #3
On 07/01/2016 10:15 AM, Zack Weinberg wrote:
> On Fri, Jul 1, 2016 at 11:11 AM, Paul E. Murphy
> <murphyp@linux.vnet.ibm.com> wrote:
>>
>> I toyed around with various levels and options, none seem to
>> a good job of handling the renames.  I'm not sure there is a
>> one size fits all solution to getting a good, concise patch.
>>
>> A thorough review might involve diffing the b_*.c file with
>> the equivalent real type file pre-patch.
> 
> Is it feasible for you to divide each patch that renames files into
> one chunk that _just_ performs renames, and another that makes content
> changes?  Obviously they would need to be squashed back together on
> landing, otherwise the tree would be broken in the
> renamed-but-not-edited changeset, but it would make it much easier to
> review.  And I realize this might be a huge amount of tedious manual
> work.

That's a substantial amount of shuffling.  Let me twist that
idea a bit further.

I think an acceptable workaround is to prepend the latter four
patches with a patch to copy the double version to the b_ prefixed
file.  It won't break anything intermittently, and preserve the
changes in git rather than the mailing list.
diff mbox

Patch

--- c/math/s_clog10f.c
+++ w/math/b_clog10.c
@@ -6,9 +6,8 @@ 
    The GNU C Library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
    License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
+   version M_LIT (2.1) of the License, or (at your option) any later version.
+The GNU C Library is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
    Lesser General Public License for more details.