Re: [Qemu-stable] [Qemu-devel] [PATCH v4 3/3] tests: start generic qemu-qmp tests

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <address@hidden> writes:
> 
> > Hi
> >
> > ----- Original Message -----
> > -snip-
> >
> >> 
> >> I'd be willing to take this as is with a suitable TODO comment
> >> explaining where we want to go with this file.  Perhaps
> >> 
> >> /*
> >>  * This program tests QMP commands that aren't interesting enough to
> >>  * warrant their own test program.
> >>  *
> >>  * TODO The tests we got here now aren't good examples, because they
> >>  * don't really exercise the commands, but only demonstrate specific
> >>  * bugs we've fixed.
> >>  */
> >> 
> >> What do you think?
> >
> > It looks like a comment that may stale. I have a few tests in some wip
> > branch that will go naturally there, so I hope it won't remain bug-fix
> > only checks. I can't say how long it will take to get there though, so I
> > am fine with a comment anyway, perhaps without TODO?
> >
> > thanks
> 
> Since you got more tests coming up, we have several workable options:
> 
> (1) Delay this patch until we got more substantial tests.  I'm wary of
>     rejecting the imperfect solution I can have now for a better
>     solution I might get some day, but since you already got something
>     better in the pipeline, I'd be happy to wait in this case.
> 
> (2) Apply it now, with my TODO.  Adding tests should eventually resolve
>     the TODO.  If we forget to delete it then, it'll go stale.  But
>     it'll be pretty obviously stale.
> 
> (3) Apply it now, without my TODO.  Until we acquire tests that would
>     resolve the TODO, the file is an unmarked bad example.
> 
> I like (1) better than (2), because it's less churn, and I don't expect
> to lose anything.  (3) my least favourite option, because I prefer maybe
> having an obviously stale TODO in the future over having an umarked bad
> example now.

Please go for 2, the other series is unrelated and I don't know when I am going 
to send it.

thanks