On Fri, 29 Aug 2014 13:18:37 -0700 Jason Gerecke <killert...@gmail.com> said:
review below. btw. it'd be really awesome if you could use arc to land these patches for review. this is part of our "getting organized" and using tools to make sure patches don't vanish in an inbox. :) there is a special limited sized review list for patches which allows inline code comments etc. etc. see http://phab.enlightenment.org http://phab.enlightenment.org/w/arcanist/ 0001-Break-_ecore_x_input_handler-into-task-specific-sub-.patch: it'd be nice if it didn't have so many whitespace changes. it'd be REALLY nice if you could "fix" the whitespace to match what was there before. i'm sifting through it and almost all of this patch is whitespace changes (that are not needed as no extra if, whole, for etc. clauses). 0002-Select-XI2-events-whenever-possible.patch: i am not sure here, but this may break multitouch support mpx-style that checks for floating slaves vs slaves. (reading the diff is not fun :) it is removing all the detection code and that is worrying, especially as it's hard to test this support (but has been tested in the past when written). 0003-Define-and-implement-new-Ecore_Event_Axis_Update-eve.patch: you have some ome todo's and fixme's... :)). one thing - the #include of xserver-properties.h ... that literally is not an existing installed file anywhere on my xorg installation. so this is going to be a problem... it won't compile. and it likely isn't going to compile for anyone else. what is this doing there and why? (oh .. and we don;'t use egyptian brackets in count_bits ()) also @since 1.12 in the doxy comments so we know what version of efl this was introduced from :) oh and typedef enum _Ecore_Axis_Label like Ecore_Event_Axis_Update was. and in fact like 0004 below - flatten out Ecore_Axis that isnt even typedeffed. :) 0004-Implement-EVAS_CALLBACK_AXIS_UPDATE-event-and-friend.patch: might i suggest not using struct _Evas_Axis * as a parameter but actually passing in the enum lable and double. this makes it easier for bindings to other languages. so pass them in as 2 params. that prbably means removing this struct entirely (also being a raw struct is inconsistent with our typedeffing of all of these) so just break out enum (actually also typedef the enum please) and double value into the event update struct :) flatten the world! :) 0005-Wire-the-Ecore-and-Evas-implementations-of-axis-upda.patch: fine EXCEPT i see no patch to expose ecore_event_evas_axis_update in the header file :) it's EAPI (exposed api)... thus shoulld have something in Ecore_Input.h :) don't forget @since 1.12 in a doxy comment :) 0006-DEBUG-Add-axis-update-logging-to-evas-multi-touch.c.patch: fine 0007-DEBUG-Make-evas-multi-touch-demo-be-pressure-sensiti.patch fine > Its been quite a while since my first patches were sent, so > unsurprisingly they no longer apply to the master branch. I've > attached an updated patchset which has been rebased and should work > once more. Hopefully it makes reviewing the code a bit more attractive > to those who've been standing on the sidelines... I'm not terribly > familiar with the EFL codebase so I'm *sure* somebody out there could > provide some feedback on the design :) > > Jason > --- > Now instead of four in the eights place / > you’ve got three, ‘Cause you added one / > (That is to say, eight) to the two, / > But you can’t take seven from three, / > So you look at the sixty-fours.... -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
