Felix Schwarz wrote: > 1. Importing a package/module instead of classes > -import trac.perm as perm > +from trac.perm import PermissionCache, PermissionSystem
In this particular case, I agree that importing the classes directly is
clearer. I wouldn't use this as a general rule, though. For example,
trac/ticket/admin.py imports the ticket model module as:
from trac.ticket import model
and references the classes as e.g. model.Milestone. In that case, I find
the current usage to be clearer.
> 2. Repeated use of string constants when you can easily use a local variable
> (see ask_cleanup_postgres_dbparams for context).
Yes, your patch improves readability. You could even assign db_params
two lines higher, replacing:
db_prop.setdefault('params', {})
with:
db_params = db_prop.setdefault('params', {})
-- Remy
signature.asc
Description: OpenPGP digital signature
