Michael DeHaan wrote:
> Vreman, Peter - Acision wrote:
>>
>> I have extended the prototype i’m working on for the system editing. 
>> Attached is a new patch that implements the following things:
>>
>> Buttons above the system list
>>
>> Support Add/Rename/Delete operations from the system list
>>
>> Support changing netboot/profile on multiple systems
>>
>> Support power control on multiple systems (requires remote power_api 
>> patch)
>>
>> Comments are welcome.
>>
>> Regards
>>
>> Peter
>>
>>
>> This e-mail and any attachment is for authorised use by the intended 
>> recipient(s) only. It may contain proprietary material, confidential 
>> information and/or be subject to legal privilege. It should not be 
>> copied, disclosed to, retained or used by, any other party. If you 
>> are not an intended recipient then please promptly delete this e-mail 
>> and any attachment and all copies and inform the sender. Thank you.
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> cobbler mailing list
>> [email protected]
>> https://fedorahosted.org/mailman/listinfo/cobbler
>
> Thanks for submitting this.
>
> I've noticed a problem with this patch in that it appears to introduce 
> some inconsistent Python tabbing. While I /love/ Python for it's 
> whitespace significance, mixing spaces and tabs can lead to problems 
> with the interpreter, so this is something that must be watched for. 
> For instance, when I'm applying this, I see the warning:
>
> [EMAIL PROTECTED] cobbler]$ git apply /tmp/system_actions.diff
> /tmp/system_actions.diff:23: space before tab in indent.
>
> In this case the patch applies as:
>
> def system_delete(self,targetlist=None,**args):
> if not self.__xmlrpc_setup():
> return self.xmlrpc_auth_failure()
> if targetlist is None:
> return self.error_page("Targetlist parameter is REQUIRED.")

Ironically, my mailreader keeps the above from displaying correctly.  
What I meant to say was the second to list line is in line with the return.


>
> As you can see, the "if targetlist is None" code can never be executed 
> because it immediately follows the return.
>
> I'd also just modify the system_delete method to require the 
> targetlist because it seems to be a code error if it's not supplied, 
> otherwise the user won't understand what a "targetlist" is.
>
> If the system delete code works out well, I take it you're going to 
> make all the other object deletes work the same way?
>
> This seems like it would be very nice for batch deletes.
>
> Thanks!
>
> --Michael
>
>
>

_______________________________________________
cobbler mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/cobbler

Reply via email to