[Bf-codereview] Use DerivedMesh instead of Mesh in COLLADA GeometryExporter (issue4387052)

dfelinto at gmail.com dfelinto at gmail.com
Tue May 3 01:05:24 CEST 2011


Some comments on syntax:
* Try to follow the file syntax (e.g. if { in oppose to if \n {).
* Also as a general rule of thumb patch should have only the essential
code on it. e.g. changes of tabs stripping in the beginning of lines
should be avoided (they led to misleading traceback of commits when
using svnblame/log.

It's always recommended to use meld (linux) or svntortoise base (win) to
clean it first.

Other than that patch LGTM. Although I'm not familiar with this area of
code.


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp
File blender/source/blender/collada/GeometryExporter.cpp (left):

http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#oldcode30
blender/source/blender/collada/GeometryExporter.cpp:30:
better not change lines if not needed. The cleaner the patch the better
(it helps svn praise/blame/log later)

http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp
File blender/source/blender/collada/GeometryExporter.cpp (right):

http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode47
blender/source/blender/collada/GeometryExporter.cpp:47:
same comment as above.

http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode127
blender/source/blender/collada/GeometryExporter.cpp:127:
avoid changes with no real change (e.g. stripping off tabs).

http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode408
blender/source/blender/collada/GeometryExporter.cpp:408:
if(!smoothnormal)	// flat
use { in the same line as if (i.e. follow file syntax)

http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode413
blender/source/blender/collada/GeometryExporter.cpp:413: {
same as above

http://codereview.appspot.com/4387052/


More information about the Bf-codereview mailing list