Dnia 2011-10-31, pon o godzinie 08:47 -0500, Chris Bagwell pisze: [..] > HI Przemo, Hi Chris,
> I thought maybe you were going to have a few more patches on > linuxwacom before forwarding... so I hadn't previously given any > nitpicking comments. > > Here they are... For most part, you can ignore these and send to > linux-input but you may get same comments from others there. Thanks for your comments - I really appreciate it! > 1) First, can you please try to get "git send-email" (or some similar > solution that doesn't have tab issues) working so that the patches are > not attachments? Its very difficult to provide review comments for > attachments. I don't think on linux-input people will bother to > review attachments. Done, thanks for the suggestion :-) The very first public test will happen soon. > 2) wacom_i4_parse_pen_report() > > + if (pressure > 1) > + input_report_key(input, BTN_TOUCH, 1); > + else > + input_report_key(input, BTN_TOUCH, 0); > + > > I think there is a preference on linux-input to make that a single statement: > > input_report_key(input, BTN_TOUCH, pressure > 1); Done > 3) wacom_raw_event() > > In wacom_raw_event(), I think majority of change are whitespace only > as you've shifted graphire logic inside a case statement. It would be > much easier for reviewer if you broke out the graphire rearrangement > part as its own patch and then have a I4 patch after that. > > In addition, its probably best to move the graphire parsing logic to > its own function; like you've done for I4. If you did that, then > there may not even be a whitespace change anymore showing up in diff > report and would be even easier to review. Good idea, done. I think it's a bit easier to read. > 4) wacom_input_mapped() > > I'd break the removal of this function into its own patch since its > not entirely related to I4 support and to make easier to review. I'd better not answer this one, but wacom_mapped_input is now back. :-) -- Regards, Przemo Firszt ------------------------------------------------------------------------------ RSA(R) Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1 _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
