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

Re: [[RFC] Netcfg add WPA support 3/4] Add WPA support to netcfg



On Fri, May 02, 2008 at 06:49:02PM +0800, Glenn Saberton wrote:
> […]

The commit log is a bit too detailed and too specific at the same time.
I am pretty sure you can find recommandations on what good commit
message should look like. (I can't give you any pointers right now,
being offline.)

> […]
>  		mkdir -p debian/$(PACKAGE)/var/dhcp ; \
> -		install -m 755 killall.sh debian/$(PACKAGE)/bin/killall.sh)
> +		install -m 755 killall.sh debian/$(PACKAGE)/bin/killall.sh; \
> +                install -m 755 killwpa.sh debian/$(PACKAGE)/bin/killwpa.sh)

Indendation issue.

> […]
> +            else {
> +                fprintf(fp, "\t# wireless-* options are implemented by the wireless-tools package\n");
>              fprintf(fp, "\twireless-mode %s\n",
>                      (mode == MANAGED) ? "managed" : "ad-hoc");

Please indent the other statements affected by a change in the control
flow.

> […]
> -#define REPLY_CHECK_DHCP             6
> -#define REPLY_ASK_OPTIONS            7

Err… this was added by your previous patch, wasn't it?  Should be
merged or fixed, then.  The rest of the patch exhibits the same kind of
errors.

Please review your patches for such kind of errors before sending them
to the list…

Cheers,
-- 
Jérémy Bobbio                        .''`. 
lunar@debian.org                    : :Ⓐ  :  # apt-get install anarchism
                                    `. `'` 
                                      `-   

Attachment: signature.asc
Description: Digital signature


Reply to: