On Nov 3, 2014, at 2:33 PM, Eitan Eliahu <elia...@vmware.com> wrote: > Thank for the review. > " Since you have declared $vmName as mandatory, is this check required?" > As I understand the "mandatory" parameter definition of PS means that the > parameter must be provided by the caller. If the parameter is not provided PS > prompts the use. > This is different than checking that the parameter is not zero.
Thanks for the explanation. I suppose you are trying to handle the case where the name of the VM is "". In that case, maybe it is better to add a: ValidateLength(1, MAXLEN), where MAXLEN can be any large length like 2048. Code code as it stands, does not handle "" very well anyway. It accesses $vnet[0] with $vnet = 0. It does not even throw an error when vm name is "". > " Here, there's an assumption that we'll set it only on the first VIF and > ignore the rest. Is that the right thing? Can we document this somewhere?" > Yes, it assumed a single VNIC per VM. I thought to document it but I didn't > find a single comment on the whole script :-) > Perhaps, I will add the comment to the commit message. We were in a hurry earlier, hence no comments :) Looks like adding comments is very straight forward: http://msdn.microsoft.com/en-us/library/dd901838(v=vs.85).aspx -- Nithin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
