From: Thien-Thi Nguyen <ttn@gnuvola.org>
To: ludo@gnu.org (Ludovic Courtès)
Cc: guile-devel@gnu.org
Subject: Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-11-73-gedb3cfc
Date: Wed, 16 Jun 2010 00:22:45 +0200 [thread overview]
Message-ID: <877hm0rl56.fsf@ambire.localdomain> (raw)
In-Reply-To: <87pqzsj970.fsf@gnu.org> ("Ludovic Courtès"'s message of "Tue, 15 Jun 2010 23:08:03 +0200")
() ludo@gnu.org (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?
> +@deffn {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
next prev parent reply other threads:[~2010-06-15 22:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <E1OOUGG-0001JC-VC@vcs-noshell.in.savannah.gnu.org>
2010-06-15 21:08 ` [Guile-commits] GNU Guile branch, master, updated. release_1-9-11-73-gedb3cfc Ludovic Courtès
2010-06-15 22:22 ` Thien-Thi Nguyen [this message]
2010-06-15 22:35 ` Ludovic Courtès
2010-06-16 7:25 ` Andy Wingo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877hm0rl56.fsf@ambire.localdomain \
--to=ttn@gnuvola.org \
--cc=guile-devel@gnu.org \
--cc=ludo@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).