On 1/10/13 4:15 AM, Olemis Lang wrote:
I commited patches in r1430768.
I see . I'll try to explain something we've being doing since long
time ago (based on Gary's suggestions) and I think is very helpful .
Commits should be focused considering their intent . The more focused
the better . That's exactly why I developed three patches for
(related) features : 1. product environments , 2. product config , 3.
test code . Therefore each patch should be applied in a separate
changeset (e.g. like in #146 [1]_ )
You might say «that's insane» ... but take as example #341 [2]_ which
is a regression after #146 . As I've also followed the same approach
to build the patches I submit , it's possible to identify the exact
point [3]_ where this very same issue was fixed once upon a time (...
so let's all hail Gary ! )
Ok, thanks for the explanation, sounds reasonable, will follow these
guidelines in the future ;)
I started integrating ProductEnvironment with the database proxy (#288),
good .
environment factory (#322)
I'll take a look to see what's been done about this ... is there
anything already in svn repos ?
and global hooks (#323).
:)
All the work done in integrating #115 with #288 and work done on #322
and #323 is in the svn repo on bep_0003_multiproduct branch.
A question re ProductEnvironment class - is there a specific reason for
that class not being derived directly from trac.env.Environment?
We've been talking about this via #bloodhound @ freenode . Could you
please post a summary ?
Short summary would be that even though trac.env.Environment and
ProductEnvironment share the same interface they are semantically
different. That and because the intent is to keep the product
environments transient (lightweight) they're implemented as a separate
class (and not derived from trac.env.Environment). Only methods/props
that require special handling are implemented/overriden in
ProductEnvironment, the rest is forwarded to the global
trac.env.Environment.
I'm
asking this as I was planning on replacing (monkey patching)
trac.env.Environment with our implementation of the environment. This
way we have control over database connection in all environments (global
and per product).
I've mentioned this before and mentioned that the way to go should be
to install product-specific DB proxies in each instance of
ProductEnvironment class . In order to do that (and in general)
there's no need to extend Environment class at all . ProductEnvs are
wrappers .
Making that (our env) a base class for
ProductEnvironment would make things more consistent.
I don't think so .
Agree now that some more light has been shed on the ProductEnvironments :)
Cheers,
Jure
