[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patch 1/4] Rename isnand.h to isnand-nolibm.h, similarly for isnanf

From: Ben Pfaff
Subject: Re: [patch 1/4] Rename isnand.h to isnand-nolibm.h, similarly for isnanf.h.
Date: Sat, 12 Jul 2008 12:00:52 -0700
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux)

Thanks for the review, Bruno.

Bruno Haible <address@hidden> writes:

>> I broke up the monolithic
>> patch into four smaller patches as you requested.
> Thanks a lot; it is much easier to review this way.
> Part 1 is perfect (just an extraneous space in the ChangeLog entry).

I couldn't spot the extra space, so I left the ChangeLog entry as-is.

(Also, I am reluctant to ever call software "perfect".)

> Part 2: In m4/isnand.m4 the variable gl_func_isnand looks unused; can you add
>         a comment saying that it's used by isnan.m4?


>         The body of gl_BUILD_ISNAND has too much indentation. The
>         last line of macro gl_HAVE_ISNAND_NO_LIBM is also too much indented.
>         But otherwise it looks perfect.

Both fixed.

> Part 3: The body of gl_BUILD_ISNANF has too much indentation. Otherwise 
> perfect
>         as well.


> Part 4: The last line of the ChangeLog entry is extraneous.
>         The body of gl_BUILD_ISNANL has too much indentation.
>         gl_ISNAN needs to start with AC_REQUIRE([gl_MATH_H_DEFAULTS]) because
>         it sets REPLACE_ISNAN, assuming that REPLACE_ISNAN has its default
>         value set before.

All fixed.

> Great work! Please apply these 4 patches.

Thanks.  With those changes, I pushed this to the repository.
"Sanity is not statistical."
--George Orwell

reply via email to

[Prev in Thread] Current Thread [Next in Thread]