On Saturday 27 October 2007 8:24:24 am James Bottomley wrote: > On Fri, 2007-10-26 at 23:05 -0700, Randy Dunlap wrote: > > On Fri, 26 Oct 2007 23:18:00 -0500 Rob Landley wrote: > > > From: Rob Landley <rob@xxxxxxxxxxx> > > > > > > Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile. > > > > I have comments for all 15 patches here. > > > > a. You should cc: the maintainer who you want/expect to apply the > > patches. Always. Andrew Morton is the only person who > > trolls for patches on a mailing list. :) > > Realistically too, for this first batch that covers mostly the mid-layer > and the transport classes, a single patch like you originally submitted > was just fine. One patch per file isn't. Patches should be split > across functional areas (in this case the function is simply adding the > SCSI mid layer to the docbook build, so they can all go together). Ooh, that makes things much easier on me. Thanks. :) > > b. The function "Description:" section header is not strictly > > required by scripts/kernel-doc. It will assume that the > > first text section after parameters is the Description: > > section. FYI. > > > > c. Extraneous whitespace. Git or quilt check for this. > > I don't know about hg... > > Actually, the checkpatch.pl script in the scripts directory of the > kernel is very good for this (I ran it on the original monolithic > patch): Ok. I'll add that to my ~/cannon.sh to put a patch in cannonical format. > In addition, there seem to be a few functions that you added kerneldoc > for which don't show up in the doc output. The one that caught my eye > was scsi_cmd_get_serial(), but I think there are others. I'm replying to Randy's message in another window as I deal with the issues he raised, but I'll cut and paste that chunk over here: These comments could use a lot more work in general. Several exported functions aren't commented at all, and several files have both internal functions and external functions with kerneldoc comments (which I'm currently grabbing only one or the other of, because the build has !I and !E which grab one of the other, and both complain and insert junk into the output if the file hasn't got any of the type it's looking for). Ideally I'd like to grab them all in one pass and annotate each function name with the export type (if any). Not just "is it exported" but is it EXPORT_SYMBOL or EXPORT_SYMBOL_GPL or whatever those EXPORT_PER_CPU_SYMBOL things are I haven't had time to look into yet. But this requires a change to the kernel build infrastructure. (I hope it can be done in docproc.c, I really don't like trying to modify other people's perl code. Anyway, todo item. Probably a new include type !B for both. Possibly I could currently combine !I and !D now, but then I wouldn't get the annotations about export type...) > James Rob -- "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
