[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] parttool
From: |
Yoshinori K. Okuji |
Subject: |
Re: [PATCH] parttool |
Date: |
Sun, 5 Apr 2009 23:48:15 +0900 |
User-agent: |
KMail/1.9.10 |
On Saturday 04 April 2009 18:22:39 phcoder wrote:
> Can someone review this patch?
Only some style should be corrected. For example:
+#ifndef GRUB_UTIL
+ if (!parts)
+ grub_dl_unref (mymod);
+#endif
Our convention is to put a space between "!" and "parts", so this should be:
+#ifndef GRUB_UTIL
+ if (! parts)
+ grub_dl_unref (mymod);
+#endif
Also, the ChangeLog should state what variables are affected explicitly. For
example:
* conf/common.rmk: Added parttool.mod and pcpart.mod
Instead, you should say:
* conf/common.rmk (parttool_mod_SOURCES): New variable.
(parttool_mod_CFLAGS): Likewise.
(parttool_mod_LDFLAGS): Likewise.
...and so on...
The code itself looks good, so you can check it in, once you correct the
styles.
Regards,
Okuji
- Re: [PATCH] parttool, phcoder, 2009/04/04
- Re: [PATCH] parttool,
Yoshinori K. Okuji <=
- Re: [PATCH] parttool, phcoder, 2009/04/06
- Re: [PATCH] parttool, Pavel Roskin, 2009/04/06
- Re: [PATCH] parttool, phcoder, 2009/04/06
- Re: [PATCH] parttool, Pavel Roskin, 2009/04/06
- Re: [PATCH] parttool, phcoder, 2009/04/06
- Re: [PATCH] parttool, Pavel Roskin, 2009/04/06
- Re: [PATCH] parttool, phcoder, 2009/04/08
- Re: [PATCH] parttool, Pavel Roskin, 2009/04/08
- Re: [PATCH] parttool, phcoder, 2009/04/17
- Re: [PATCH] parttool, Vladimir Serbinenko, 2009/04/25