On Thu, Jan 05, 2012 at 04:47:03PM +0000, Andrea Spadaccini wrote:
> Hi Iustin,
> 
> > One general comment: this doubles the number of calls (3→6) and also
> > adds them in cluster modify (so 3→12).
> 
> Cluster modify was already tested, so it's 6→12, but of course your
> point is still valid.
> 
> > Given that each separate call
> > does create a job, submit it, etc., this will increase the QA time
> > somewhat needlessly (all these could be unittests instead). I.e. it's
> > fine to run one or two parameters, but you don't need to cmd-line QA all
> > of them.
> 
> Will remove the 3 additional tests. (And will update the commit
> message in case of LGTM)

Thanks.

> > You're using tuples as list, please don't do that. If you iterate over a
> > tuple it means you're misusing it.
> 
> I'll change it to a list, even if I don't really see why this is a
> misuse of tuples ­— except for breaking the Ganeti conventions.

I'm probably more influenced by Haskell, but (as Michael said offline)
tuples are the equivalent of 'structs', rather than 'collections'.

thanks, LGTM.

iustin

Reply via email to