[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [patch] for mig check in GDB's configure
From: |
Ivan Shmakov |
Subject: |
Re: [patch] for mig check in GDB's configure |
Date: |
Fri, 03 May 2013 12:57:21 +0000 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux) |
>>>>> 陆岳 <hacklu.newborn@gmail.com> writes:
[…]
A few minor points.
> From 13d3edd1f6dbbc20b2801cea1fc367bf9042f977 Mon Sep 17 00:00:00 2001
> From: hacklu <hacklu.newborn@gmail.com>
> Date: Fri, 3 May 2013 18:27:08 +0800
> Subject: [PATCH] Patch check mig on GNU Hurd
> 2013-05-3 hacklu <hacklu.newborn@gmail.com>
There should be two spaces between the name and email, too. And
the date should be 2013-05-03, as per ISO 8601. (One may want
to check the respective Wikipedia article here.) As in, e. g.:
$ date -uI
2013-05-03
$
Also, I'd urge you to use the “real” full name -- the same as
you'd use when filing a GSoC application, or copyright
assignment papers (as required by FSF), etc.
> * configure.ac : uncorrectly check for mig on GUN Hurd
• There should be no space before “:”;
• the sentence should begin with a capital letter, and end with
a period;
• s/GUN/GNU/.
Besides, I'm having trouble understanding the intended meaning
of this sentence. Shouldn't it be, say, as follows instead?
* configure.ac: Ensure MIG is available when building for
GNU Hurd.
[…]
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2013-05-3 hacklu <hacklu.newborn@gmail.com>
> +
> + * configure.ac : uncorrectly check for mig on GUN Hurd
> + * configure: Regenerate.
> 2013-04-30 Samuel Thibault <samuel.thibault@gnu.org>
The same applies here. Also, there should be an empty line
between the ChangeLog entries.
[…]
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -488,6 +488,15 @@ AC_CHECK_TOOL(WINDRES, windres)
> # Needed for GNU/Hurd.
> AC_CHECK_TOOL(MIG, mig)
> +case "${host}" in
Note that ‘{}’ are superfluous here; case "$host" will do the
same, but is a tiny bit shorter.
> + *-linux*|*-k*bsd-gnu*)
> + ;;
> + i[?]86-*-gnu*)
> + if test "$MIG" = "" ; then
> + AC_MSG_ERROR([MIG not found but required for $host])
> + fi
> + ;;
I guess that both cases should use the same indentation level —
either 4 spaces or 8 (or a TAB.) Check the rest of the GDB .ac
code for the currently preferred style.
Also, it's possible to avoid an indentation in the first case
altogether, like:
+ *-linux*|*-k*bsd-gnu*) ;;
Again, look at the rest of the GDB code and use a matching
style.
> +esac
[…]
--
FSF associate member #7257
- [patch] for mig check in GDB's configure, 陆岳, 2013/05/03
- Re: [patch] for mig check in GDB's configure, Thomas Schwinge, 2013/05/03
- Re: [patch] for mig check in GDB's configure, 陆岳, 2013/05/03
- Re: [patch] for mig check in GDB's configure,
Ivan Shmakov <=
- Re: [patch] for mig check in GDB's configure, Yue Lu, 2013/05/04
- Re: [patch] for mig check in GDB's configure, Thomas Schwinge, 2013/05/16
- Re: [patch] for mig check in GDB's configure, Joel Brobecker, 2013/05/17
- Re: [patch] for mig check in GDB's configure, Yue Lu, 2013/05/17
- Re: [patch] for mig check in GDB's configure, Thomas Schwinge, 2013/05/17
- Re: [patch] for mig check in GDB's configure, Joel Brobecker, 2013/05/17
- Re: [patch] for mig check in GDB's configure, Yue Lu, 2013/05/17
- Re: [patch] for mig check in GDB's configure, Doug Evans, 2013/05/05
- Re: [patch] for mig check in GDB's configure, Yue Lu, 2013/05/05
- Re: [patch] for mig check in GDB's configure, Doug Evans, 2013/05/05