Re: Wacom Calibration - Phase 1

[Jason Gerecke <killertofu gmail com>]

On Mon, Jan 9, 2012 at 7:52 AM, Bastien Nocera <hadess hadess net> wrote:
> On Mon, 2012-01-09 at 15:27 +0000, Bastien Nocera wrote:
>> Hey Jason,
>>
>> On Wed, 2012-01-04 at 18:11 -0800, Jason Gerecke wrote:
>> > (Moved to the gnomecc-list)
>> >
>> > The first of three phases on adding calibration into the Wacom panel
>> > is now "complete". That is to say, the code is working, but not
>> > necessarily pretty! I'd be most appreciative if anyone could review
>> > the changes and give me some feedback. The changes to g-c-c itself are
>> > pretty minor, but I don't doubt I've done a couple of stupid things :D
>> >
>> > The "calibration" branch at
>> > https://github.com/jigpu/gnome-control-center has the changes to
>> > g-c-c. If you want to see the gory changeset for xinput_calibrator
>> > (instead of just the commit in g-c-c where I copy the files over),
>> > it's available in the "stripped" branch of
>> > https://github.com/jigpu/xinput_calibrator Note that these are
>> > development branches -- I'll be rebasing and changing history as I
>> > please ;) Work on phase 2 (multiple monitor support) begins tomorrow!
>>
>> Could you please upload 2 separate patches to the GNOME Bugzilla, one
>> mega-patch that adds the calibrator to the tree, and one with the panel
>> integration code?
>>
>> I'll review both and merge as soon as possible, so that we can carry on
>> working upstream on this.
>
> I'll add that:
> - the calibrator code needs to be moved into a sub directory
> - the horror that is run_gui() needs to be changed, we can't have it
> blocking the UI
The mockup struck me as being modal, so I wrote run_gui under that
assumption. If its not supposed to be modal (not sure what use that
would serve, but then again, that may be part of why I suck at writing
GUIs) I can see how that'd be a problem.

> - the copy/pasted code is in severe need of a clean up
I'll agree, but I need a little more direction than that :D Are we
talking cleaner (i.e. more GNOME-like) style? Cleaner (i.e.
simplified) functions? Cleaner (i.e. unapplicable functions removed)
files? I've lost most of my perspective of what needs attention after
mucking through the trenches with the C++ to C conversion.

> - the code in cc-wacom-page.c needs to use the same coding style
> (indentation) as the rest of the existing code
> - s/axys/axis/
>
> Cheers
>

Do you want me to make these changes before uploading the patches, or
after they're upstream? I'm all for getting the code upstream as
quickly as possible (so some of this work can be parallelized and not
block on my learning curve), but I'm not sure how cooked you want the
patches.

Jason

---
Day xee-nee-svsh duu-'ushtlh-ts'it;
nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
Huu-chan xuu naa~-gha.