> On 2009-04-10 18:43:48.042762, Paul Wise wrote: > > Patch looks good, except for a few remaining comments from the previous > > version and adding copyright info and adding your name to AUTHORS. > > > > You might also want to add doxygen comments for the new public functions. > > > > I assume it compiles and you've tested it? Please mention that in the > > testing done section. > > Gerco Ballintijn wrote: > Added some text on testing.
OK, with the below changes I think this is ready to commit, please do so. > On 2009-04-10 18:43:48.042762, Paul Wise wrote: > > synfig-studio/trunk/src/gtkmm/devicetracker.cpp, line 8 > > <http://patches.synfig.org/r/19/diff/2/?file=119#file119line8> > > > > You have added a fair bit of code, please add your copyright here like > > this: > > > > Copyright 2009 Gerco Ballintijn > > > > Please also add yourself to the AUTHORS file. You are already in > > about.cpp so that doesn't need changing. > > wrote: > Done. I forgot to say, please also update your copyright years in the README file. > On 2009-04-10 18:43:48.042762, Paul Wise wrote: > > synfig-studio/trunk/src/gtkmm/devicetracker.cpp, line 33 > > <http://patches.synfig.org/r/19/diff/2/?file=119#file119line33> > > > > Please file a bug on GTKmm asking for the required features to be added > > and add a comment and link about it here. > > wrote: > I confirmed this idea by looking at the actual wrapper code for > Gtk::Device, and reading a TODO entry for the required functionality in > there. So I guess the GTKmm already know some extra work is needed. Fair enough. Maybe add a FIXME comment here so we have a pointer to it, and file a bug about it too once this is committed. > On 2009-04-10 18:43:48.042762, Paul Wise wrote: > > synfig-studio/trunk/src/gtkmm/devicetracker.cpp, line 59 > > <http://patches.synfig.org/r/19/diff/2/?file=119#file119line59> > > > > Should this comment be changed too? > > wrote: > I think these comments should simple be removed. The commented-out code > was basically wrong, so it should have been removed completely (not just > commented out). With this patch initializing is done via the DeviceSettings > class. The comment is no longer relevant. Should this also be a separate > patch? OK, lets remove it, no need to submit a patch for that though. > On 2009-04-10 18:43:48.042762, Paul Wise wrote: > > synfig-studio/trunk/src/gtkmm/devicetracker.cpp, line 80 > > <http://patches.synfig.org/r/19/diff/2/?file=119#file119line80> > > > > Whitespace removal, please undo this. > > wrote: > This appears to be a bug in the patch review software since this line is > not in the original patch file. Hmm, true. > On 2009-04-10 18:43:48.042762, Paul Wise wrote: > > synfig-studio/trunk/src/synfigapp/inputdevice.cpp, line 283 > > <http://patches.synfig.org/r/19/diff/2/?file=121#file121line283> > > > > This change belongs in a separate patch since it has nothing to do with > > device settings saving. > > wrote: > Added separate patch for review. Great. - Paul ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://patches.synfig.org/r/19/#review34 ----------------------------------------------------------- On 2009-04-11 03:01:13.096969, Gerco Ballintijn wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://patches.synfig.org/r/19/ > ----------------------------------------------------------- > > (Updated 2009-04-11 03:01:13.096969) > > > Review request for Synfig. > > > Summary > ------- > > This a preliminary patch that adds the capability of saving and restoring the > input devices configuration (i.e., tablet configuration). > > Note: The code specifically doesn't use the GTKmm layer but uses GTK+ > directly since the GTKmm wrapper is incomplete. > > There are a couple of I don't like about the patch, but first I await your > comments... > > > Diffs > ----- > > synfig-studio/trunk/src/gtkmm/app.cpp 2357 > synfig-studio/trunk/src/gtkmm/devicetracker.h 2357 > synfig-studio/trunk/src/gtkmm/devicetracker.cpp 2357 > synfig-studio/trunk/src/synfigapp/inputdevice.h 2357 > synfig-studio/trunk/src/synfigapp/inputdevice.cpp 2357 > > Diff: http://patches.synfig.org/r/19/diff > > > Testing > ------- > > Synfigstudio compiles with the patch. Changes to an input device (e.g., the > stylus) are persistent over normal restart. Changes are visible in the > settings file. Configuration of key event generation (i.e., the pad input > device) is maintained over restarts, but I don't really know how to use them, > so I didn't test this functionality. > > > Thanks, > > Gerco > > ------------------------------------------------------------------------------ This SF.net email is sponsored by: High Quality Requirements in a Collaborative Environment. Download a free trial of Rational Requirements Composer Now! http://p.sf.net/sfu/www-ibm-com _______________________________________________ Synfig-devl mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/synfig-devl
