2013/9/14 Krzysztof Kosiński <tweenk.pl@...400...>:
2013/9/14 Markus Engel <p637777@...1081...>:
Hi everyone,
have you worked your way through my changes yet? Any comments? J
Krzysztof, you wanted to do the review?
The changes seem mostly OK, but there are a lot of conflicts with the current trunk. It would help a lot if you updated the branch so that it merges cleanly.
Oops, I didn't notice you already did that. :)
Here are some initial comments:
1. There are several coding style problems: - Whitespace does not conform to coding standards. We always use 4 spaces as the indent, while some of the files you've changed have inconsistent indent (sometimes 4 spaces, sometimes 1 tab). - We always write the reference and pointer signs attached to the variable name, not the type name. - We qualifiers such as const after the type name, not before it, because they bind to the right.
The first two problems can be fixed automatically by running astyle, but const placement needs to be fixed manually.
For reference: http://staging.inkscape.org/en/develop/coding-style/
2. SP_IS_TYPE() macros should be implemented as a comparison with NULL, so that you can't assign the result to a pointer. SP_TYPE() macros should be reimplemented as dynamic_cast so that they generate compile-time errors when something not derived from SPObject is passed.
Other than that I haven't found any significant problems yet; the code looks mergeable. I'll complete the review today or tomorrow.
Regards, Krzysztof