Mailing-List: contact bloodhound-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: bloodhound-dev@incubator.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
Message-ID: <50F033E2.4050509@digiverse.si>
Date: Fri, 11 Jan 2013 16:46:42 +0100
From: Jure Zitnik <jure@digiverse.si>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7;
 rv:17.0) Gecko/17.0 Thunderbird/17.0
MIME-Version: 1.0
To: bloodhound-dev@incubator.apache.org
Subject: Re: [Apache Bloodhound] #115: Product-specific settings
References: <055.55b84508f6fcc1a8cc545a03456faa21@incubator.apache.org>
 <070.15b8ebe71bafa7ae651d57d105b756d7@incubator.apache.org>
 <CAGMZAuM635pU5wQ=0-f-_5=TS=-Oy3-J0r34BrQ75TWpc=A4HQ@mail.gmail.com>
 <50D42A23.20005@digiverse.si>
 <CAGMZAuPO9N_fBbv=dCa3taUGVJuvitVVtyQ3sMVd5UCOK1T1RQ@mail.gmail.com>
 <50ED4B83.7050208@digiverse.si>
 <CAGMZAuMy1GbvR1DT1DjKVsDte4QVnwVii+PTnVnC6y3M-p3zvQ@mail.gmail.com>
In-Reply-To: 
 <CAGMZAuMy1GbvR1DT1DjKVsDte4QVnwVii+PTnVnC6y3M-p3zvQ@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

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