Revision: 65185
http://sourceforge.net/p/brlcad/code/65185
Author: brlcad
Date: 2015-06-04 20:23:52 +0000 (Thu, 04 Jun 2015)
Log Message:
-----------
stub in an initial file that outlines common steps that are taken during code
review. these is literally just the list of all the things I think about when
reviewing patches, so there's undoubtedly room for improvement. it's a step,
though, towards better formalization and automation.
Modified Paths:
--------------
brlcad/trunk/doc/CMakeLists.txt
Added Paths:
-----------
brlcad/trunk/doc/code_review.txt
Modified: brlcad/trunk/doc/CMakeLists.txt
===================================================================
--- brlcad/trunk/doc/CMakeLists.txt 2015-06-04 20:11:45 UTC (rev 65184)
+++ brlcad/trunk/doc/CMakeLists.txt 2015-06-04 20:23:52 UTC (rev 65185)
@@ -27,6 +27,7 @@
benchmark.tr
brep.txt
BRL-CAD.bib
+ code_review.txt
cvs.txt
description.txt
ged.tr
Added: brlcad/trunk/doc/code_review.txt
===================================================================
--- brlcad/trunk/doc/code_review.txt (rev 0)
+++ brlcad/trunk/doc/code_review.txt 2015-06-04 20:23:52 UTC (rev 65185)
@@ -0,0 +1,48 @@
+BRL-CAD Code Review
+===================
+
+...what to look for during a review...
+
+Legal:
+ * proper copyright statement and compatible license?
+ * any indication contribution disagrees with copyright assignment?
+ * if incorporating 3rd party code, prefer bsd/mit-style options
+
+Architecture:
+ * proper location in hierarchy?
+ * proper file name(s)?
+ * is a dependency being introduced?
+ * is a cyclic dependency being introduced?
+ * are any global variables being introduced?
+
+Design:
+ * is public API minimized?
+ * is a platform symbol being used?
+ * does the change reduce or violate modularity?
+ * is the approach being taken reasonable?
+ * is the code easy to understand?
+
+Testing:
+ * compiles cleanly?
+ * are warnings handled cleanly?
+ * any blatant logic errors?
+ * are all data inputs checked properly, sanitized?
+ * if fixing a bug more than once, is there a test?
+ * is a unit test warranted?
+ * are there major robustness issues?
+
+Documentation:
+ * if publicly visible, is NEWS updated?
+ * if public API changed, is CHANGES updated?
+ * if public API introduced, does every symbol have a comment?
+ * if interface changed, are man page or other docs updated?
+ * are numeric constants documented?
+ * is suspicious, complex, risky code commented?
+
+Code:
+ * conforms with style and coding standards?
+ * proper indentation, formatting, whitespace?
+ * are symbol names consistent?
+ * is duplication being introduced, refactoring warranted?
+ * is there undocumented dead / commented-out code?
+ * can any code be replaced with library calls?
Property changes on: brlcad/trunk/doc/code_review.txt
___________________________________________________________________
Added: svn:mime-type
## -0,0 +1 ##
+text/plain
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
------------------------------------------------------------------------------
_______________________________________________
BRL-CAD Source Commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/brlcad-commits
