On Jul 4, 2010, at 12:55 AM, Krzysztof Kosiński wrote:
I don't want to be overly critical but so far the changes amount to mechanical replacement of "return_type sp_foo_method(SPFoo *, args...)" with "return_type method(args...)" and replacing "struct SPFoo" with "class SPFoo". This includes deprecated methods like sp_item_invoke_bbox_full with NRRect arguments. It would be more useful to identify a few places in the code with a dependency on GObject and rewrite them to eliminate that dependency.
Not really.
There are some subtle modularity and encapsulation changes that are also going in. Those are easy to miss on the surface.
And, no, it's not more useful to target GObject at this time. There are far more than just "a few", and more importantly there are subtle *logic* issues with different approaches that need to change.
Three other problems:
- Why are sp_foo_method left commented out at the bottom of the headers?
Simple working practice. He's been keeping things in as he roughs in the work, and will remove them later. He finds it easier to work on the new changes while having the old code there to refer to.
- Abhishek's changes use tabs instead of 4 spaces.
A very minor cosmetic issue that is simple to address later on. Getting things functional is far more important than a simple M-x untabify being left for later.
- New methods do not follow coding guidelines (should be camelCase
instead of lowercase_with_underscores).
Yes, this has been intentional. He has been planning on tuning the names later on. At the moment leaving some of those as-is brings several benefits. Among those are easier merging and more helpful visual diffs.