guile-devel
[Top][All Lists]
Advanced

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

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-11-73


From: Thien-Thi Nguyen
Subject: Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-11-73-gedb3cfc
Date: Wed, 16 Jun 2010 00:22:45 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)

() address@hidden (Ludovic Courtès)
() Tue, 15 Jun 2010 23:08:03 +0200

   This file is from Gnulib.  So get the patch accepted in Gnulib and it
   will get in next time we update our Gnulib copy.

Done (commit e0fcde8130473202a4dd37c41a3c331fc5a9907d)!

   OK.  How about moving the AC_DEFUN in ‘acinclude.m4’?

Sure.

   Also, the comment above should say what the macro does, rather than how
   it could possibly do it in the future.

I'd say both kinds of info have their place in the comments.  In this
particular case, the macro is so simple, i did not think to include a
proper description (plus, the definition is immediately prior to its use).
However, moving its definition away from its use is a good time to add such.

   The ‘--ttn’ can also be omitted since we’d rather communicate by email
   than via comments in ‘configure.ac’.  :-)

That's a personal style point, i'd say.  Do you really insist on its removal?

   > address@hidden {Scheme Procedure} tmpfile
   > [...]

   I suggest adding this:

     (@pxref{Temporary Files,,, libc, The GNU C Library Reference Manual}).

Yes, good idea.

   > always returns the symbol @code{tmpfile}.

   What about returning #f instead?  That would seem more consistent since
   currently ‘port-filename’ can only return either #f or a string AFAIK.

I spewed a bit on this in my reply to Andy.  Gist: meh.

   > +  FILE *rv;
   > +
   > +  if (! (rv = tmpfile ()))

   Rather like this (info "(standards) Syntactic Conventions"):

     rv = tmpfile ();
     if (rv == NULL)
       ...

OK.

   Modulo these comments, feel free to commit.  If you’re now confident
   with rebase et al., you can commit to ‘master’.  Otherwise I can pick
   them later from your branch and push them to ‘master’.

Thanks for the review (even the stuff i hadn't meant to push).  I'll stick
with commiting to ttn/* for the time being, until i get more practice.

thi



reply via email to

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