[Ovirt-devel] Re: [PATCH server] comprehensive / up to date validations patch (revised again)

[Steve Linabery]

On Thu, Jan 15, 2009 at 03:21:06PM -0500, Mohammed Morsi wrote:
> (revisions relate to small validation bugs in host, nic, task models
>   and host-browser module)
> 

snip

>  
>    # Ensures that, if the VM does not contain the Cobbler prefix, that it
> -- 
> 1.6.0.6
> 

Line 379, maybe say "MAC address" just for clarity's sake in output if it fails:
+    :message => 'A mac must be specified.'

Typo at line 569:
+         (not lvm_storage_pool.nil? and not lvm_stoage_pool.movable?)

and 606-607 should read "Task ends before it's started" (grammar police):
+    errors.add("time_ended", "Tasks ends before its started") unless time_ended.nil? or time_started.nil? or time_ended > time_started
+    errors.add("time_started", "Tasks starts before its created") unless time_started.nil? or created_at.nil? or time_started > created_at

Inconsistent indentation in boot_type_test.rb...and elsewhere...(I told you it was minor nitpicky stuff :)

So, conditional ACK, but as we discussed, a few times I found myself thinking "Well, that validation *seems* to make sense, but I don't know the model well enough to be 100% certain."

But it works and all unit tests passed.

--Steve