[ditrack commit] r839 - in src/trunk/DITrack: . Command

Vlad Skvortsov vss at 73rus.com
Mon Nov 20 10:17:13 PST 2006


oleg at ditrack.org wrote:

>+	    if e in db.cfg.filters:
>+		f = db.cfg.filters[e]
>+	    else:
>+		try:
>+		    f = DITrack.DB.Filter(e)
>+	        except:
>+		    DITrack.Util.common.err("ERROR: '%s' is not predefined filter or it contains syntax error" \
>+			% e)
>  
>
No need for '\' to continue the line, since it's inside the parenthesis.

>Modified: src/trunk/DITrack/DB.py
>===================================================================
>--- src/trunk/DITrack/DB.py	2006-11-15 07:17:47 UTC (rev 838)
>+++ src/trunk/DITrack/DB.py	2006-11-15 20:54:19 UTC (rev 839)
>@@ -67,6 +67,10 @@
>     message = CorruptedDBError.message \
> 	+ ": unparseable version sets configuration"
> 
>+class CorruptedDB_UnparseableFiltersError(CorruptedDBError):
>+    message = CorruptedDBError.message \
>+	+ ": unparseable filter configuration"
>+
>  
>
It's worth adding a testcase for filters configuration parsing (see 
similar testcases for versions, users and categories).

>+#class FilterUnknownError(Exception):
>+#    message = "Unknown predefined filter name"
>  
>
Generally one should not comment out things (just remove it!): we have 
version control system to be able to retrieve older versions, right?

> 	    if s in self.items:
>                 raise CorruptedDB_UnparseableFiltersError("Duplicate filter "
>-		    "set '" + s + "' definition")
>+		    + s + "' definition")
>  
>
I would also recommend trying to make commits as small as possible: it's 
easier to review the code if, for example, all typo/minor fixes go in 
separate commits while larger changes go in other set of commits.

Otherwise, this seems to be a move into right direction! Keep it going! :-)

-- 
Vlad Skvortsov, vss at 73rus.com, http://vss.73rus.com



More information about the Dev mailing list