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

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

[Ludovic Courtès]

Hi!

"Thien-Thi Nguyen" <ttn@gnuvola.org> writes:

> commit 0faca15c33cfbe1ef25fd6b4bbc7f92b57659345
> Author: Thien-Thi Nguyen <ttn@gnuvola.org>
> Date:   Mon Jun 14 21:57:01 2010 +0200
>
>     Init shell var properly in git-version-gen.
>     
>     * build-aux/git-version-gen: Ensure shell var
>     is not influenced by an env var of the same name.

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

> -AC_CONFIG_FILES([test-suite/standalone/test-fast-slot-ref],
> -                [chmod +x test-suite/standalone/test-fast-slot-ref])
>  AC_CONFIG_FILES([doc/ref/effective-version.texi])
>  
> +dnl We can get fancy with m4sugar (m4_foreach et al) later.
> +dnl NB: We don't jam everything into one GUILE_CONFIG_SCRIPT call
> +dnl since that expands "chmod +x LONG-LIST-OF-FILES" multiply.  --ttn
> +AC_DEFUN([GUILE_CONFIG_SCRIPT],[AC_CONFIG_FILES([$1],[chmod +x $1])])
> +
> +GUILE_CONFIG_SCRIPT([check-guile])
> +GUILE_CONFIG_SCRIPT([benchmark-guile])

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

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

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

> +@deffn {Scheme Procedure} tmpfile
> +@deffnx {C Function} scm_tmpfile
> +Return an input/output port to a unique temporary file
> +named using the path prefix @code{P_tmpdir} defined in
> +@file{stdio.h}.

I suggest adding this:

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

> +The file is automatically deleted when the port is closed
> +or the program terminates.
> +
> +The name of the temporary file associated with the returned
> +port is not available to Scheme programs;
> +@code{(port-filename (tmpfile))} 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.

> +SCM_DEFINE (scm_tmpfile, "tmpfile", 0, 0, 0,
> +            (void),
> +            "Return an input/output port to a unique temporary file\n"
> +            "named using the path prefix @code{P_tmpdir} defined in\n"
> +            "@file{stdio.h}.\n"
> +            "The file is automatically deleted when the port is closed\n"
> +            "or the program terminates.\n"
> +            "\n"
> +            "The name of the temporary file associated with the returned\n"
> +            "port is not available to Scheme programs;\n"
> +            "@code{(port-filename (tmpfile))} always returns the\n"
> +            "symbol @code{tmpfile}.")
> +#define FUNC_NAME s_scm_tmpfile
> +{
> +  FILE *rv;
> +
> +  if (! (rv = tmpfile ()))

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

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

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!

Ludo’.

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

[Thien-Thi Nguyen]

() 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

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

[Ludovic Courtès]

Hi!

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

>    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?

More importantly, I insist on reaching a consensus.  :-)

There are places in the old C code base where people really seemed to be
talking to each other via comments.  That seems misplaced to me.
Development discussions should take place on this mailing list IMO.

Thanks,
Ludo’.

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

[Andy Wingo]

On Wed 16 Jun 2010 00:35, ludo@gnu.org (Ludovic Courtès) writes:

> Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>
>>    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?
>
> More importantly, I insist on reaching a consensus.  :-)
>
> There are places in the old C code base where people really seemed to be
> talking to each other via comments.  That seems misplaced to me.
> Development discussions should take place on this mailing list IMO.

I agree. If a comment matters, it has to be maintainable, so it should
be a statement about code and not between people. I don't want to feel
that I can't edit a comment because it's not from me ;)

Andy
-- 
http://wingolog.org/