On Friday 02 May 2008, Glenn Saberton wrote: > Hopefully I have addressed the issues pointed out so far. The first > two patches don't change netcfg's behaviour, the third adds wpa > support and the last kills it with kill() rather than the shell > script. I will work on adding functionality to allow netcfg to tell > wpasupplicant which driver to use. I would also like to add a function > that scans for access points and lets the user chose one from a list. > Thoughts? AFAICT your patches assume that wpasupplicant is available. Do you also have a patch to add a udeb for that? If so: - what size is it (both the udeb itself and when installed)? - please send that patch for review as well. I'm not sure whether we should maybe check whether wpasupplicant is available or not and only offer the option to use WPA if it is. General comment. Your patches still have way too many random whitespace/indentation changes (see my reply to patch 3). Please make sure you clean those out before submitting your next series of patches. When working on existing files you should check what its current indentation style is and follow that: if it uses 4 spaces, then always use 4 spaces; if it uses 4 spaces for half-indents and tabs for full indents, then use that. If the current file is a mess, then either - follow the indentation used in the directly surrounding lines as best as you can, or - submit a *separate* cleanup patch for the whole file Do you perhaps have tab width set to 4 positions in your editor? If that is the case, then you should change it to 8. Everybody else who sees your patches will be much happier. Make sure you never introduce new "trailing whitespace" (spaces or tabs at the end of lines). See also installer/doc/devel/codingstyle.txt in the D-I repo. Cheers, FJP
Attachment:
signature.asc
Description: This is a digitally signed message part.