Request for patch review

[Lukasz Zalewski]

On 12/10/12 15:56, Jelmer Vernooij wrote:
> Hi Lukasz,
Hi Jelmer, Andrew
>
> On Fri, 2012-10-12 at 10:16 +0100, Lukasz Zalewski wrote:
>> On 12/10/12 09:53, Lukasz Zalewski wrote:
>>> Hi all,
>>> To put the new proposed rules for patch reviews to the test i would like
>>> to ask for a review of patches that i have sent to the list (on
>>> 25/06/12) with subject:
>>> [PATCHES] Make add_remove_group_members a private helper function and
>>> partial fixes for Bug #8891
>>>
>>> They are also attached to the Bug #8891.
>>
>> Patches attached
> Thanks for working on this.
>
> These patches look good overall. Some quick comments:
>
> * As Andrew says, some tests for the new methods would be really nice.
>
> 0002-samdb-Better-exception-handling-in-_add_remove_group:
> * Please don't remove the FIXME's - they are still valid
> * When raising errors in a cmd_* implementation, use CommandError rather
> than Exception
> * self.transaction_start() should be outside of the try/finally - if it
> fails we don't want to abort the transaction
>
> 0003-s4-netcmd-group.py-throw-exception-when-nonexistent-.patch:
> * When raising errors in a cmd_* implementation, use CommandError rather
> than Exception

Thanks for the comments, shall make the changes

L