Thanks for taking a look Dan.

Dan Price wrote:
> On Tue 29 Jul 2008 at 11:55AM, Brock Pytlik wrote:
>   
>> Can someone take a quick look at this set of changes. It fixes bug 2693, 
>> changes the server catalog to have required and optional directories 
>> like the image does, and removes a print statement that was filling the 
>> logs.
>>
>> CR:
>> http://cr.opensolaris.org/~bpytlik/ips-2693/
>>     
>
> Looks good overall.  A few minor issues I spotted:
>
> catalog.py: assert not (readonly and rebuild)
> might be slightly clearer
>   
agreed
> depotcontroller.py: can you commonize with the routine above? Seems like
> a lot of duplicated code.
>   
I pulled the first half of the function out. If you really want the loop 
commonized as well, I'll pull it out, but I think it'll make the code 
impenetrable (at least the way that I can think of to do it).
> t_depotcontroller.py: should be self.assert_() instead of naked
> assert.  (The reason is that assert can get optimized out, and
> self._assert() will never be).
>   
Oops, hadn't realized that. I've made the change in t_search.py as well 
which had used assert (all the tests still pass).

New cr at: http://cr.opensolaris.org/~bpytlik/ips-2693-v2/

Thanks,
Brock
>         -dp
>
>   

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to