Marc-André Lureau <mlur...@redhat.com> writes: > Hi > > ----- Original Message ----- >> Marc-André Lureau <mlur...@redhat.com> 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
Done. Also: rename to tests/qmp-test.c, cover in MAINTAINERS.
