On Wed, Sep 09, 2009 at 05:20:20PM -0400, Ryan Leavengood wrote: > On Wed, Sep 9, 2009 at 4:00 PM, <pete.goodeve@xxxxxxxxxxxx> wrote: > > I tried... (:-/) > Just some quick suggestions: > > Things you should definitely change: > > - The copyright header style you used is still not in line with our > coding guidelines. Remove all the extra * characters. OK -- wasn't sure if that was verboten [and I thought it looked quite nice! (:-)] > - There are also extra spaces in some array indexing (like_this [i]) > and that is just plain hard to read, so please fix that. Blech -- I never noticed that. Takes another eye... > > Things that would be good (maybe in a separate style fixing patch): > > - You could fix the header order in the files you touched to match the > guidelines. I assume you're talking about devlist.c? [Which I didn't expect to touch at all, but then I changed some identifiers...] If the order is not correct in usb_midi.(c|h) please elaborate. > - You could fix the function style too (return type on its own line, > two lines between functions.) Again I thought I'd checked all those, but forgot about devlist. > - You could remove the space between a function and its first > parenthesis (on both function definitions and calls.) > > I know a lot of the above existed before, but we generally want to try > to fix style when working on something. Hopefully over time all the > code will be updated. On the new code you added the style seems pretty > good, with the exception of a few extra spaces in some if statements > and casts. OK. Thanks for the detailed look. I'll just do a do-over on those bits and build a new patch. > > Oh and thanks for this work, I have a USB MIDI keyboard too and it > will be very nice to be able to use it in Haiku, once all the kinks > are worked out. In fact I could help test this if you want (maybe once > the alpha is out.) Great! Yeah, no hurry... (:-)) I have it working on my box, which is the main thing. Cheers, -- Pete --