[Kde-games-devel] Moving palapeli-goldberg-slicer to kdereview
Johannes Loehnert
loehnert.kde at gmx.de
Tue Jul 13 21:45:42 CEST 2010
Hello,
> The code looks good to me. The algorithms could probably use some more
> documentation (just in case you get hit by a bus), but that's not a reason
> to fail the review IMO.
Well, the generators (which are the most complex functions) are all knit by
the same pattern - generate borders, collision check, generate pieces. The
most important piece of documentation from my point of view are the .svg
files. But I admit, I am a lazy code commenter.
> You did not use the SlicerMode interface provided by libpala. This is okay
> because I added these only one week ago. I have now ported the Goldberg
> slicer to use modes (r1148763).
One remark: I noticed that the "dummy entry" for irregular grids (when
qvoronoi is missing) is no longer added. I intented it to be a pointer for the
user ("there is more, but..."). Could it be added again, or would that collide
with the new wizard interface?
> To reflect the target status as the default slicer plugin, and in order to
> dejargonize the name (i.e. nobody knows what Goldberg means), I have
> renamed the slicer from "Goldberg" to "Palapeli Slicer Collection
> (formerly known as Goldberg slicer)" (r1148770). I hope that's okay for
> you.
No objections here...
Best regards,
Johannes
More information about the kde-games-devel
mailing list