qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu devel list <qemu-devel@nongnu.org>,
	Fam Zheng <famz@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Max Reitz <mreitz@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>,
	Stefan Hajnoczi <stefanha@gmail.com>
Subject: Re: [Qemu-devel] [PATCH v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns
Date: Fri, 2 Dec 2016 17:40:59 -0500	[thread overview]
Message-ID: <840cbc6c-39f6-1b66-7ceb-f7caeae4fbf2@redhat.com> (raw)
In-Reply-To: <4278883b-1567-45c7-b76c-9dcdeb577422@redhat.com>



On 12/02/2016 04:54 PM, Laszlo Ersek wrote:
> On 12/02/16 22:16, Eduardo Habkost wrote:
>> On Fri, Dec 02, 2016 at 10:01:52PM +0100, Laszlo Ersek wrote:
>> [...]
>>> +docs/*
>>> +*.txt
>>> +configure
>>> +GNUmakefile
>>> +makefile
>>> +Makefile*
>>> +*.mak
>>> +qapi-schema*.json
>>> +qapi/*.json
>>> +include/qapi/visitor.h
>>> +include/qapi/visitor-impl.h
>>> +scripts/qapi.py
>>> +scripts/*.py
>>> +*.h
>>> +qapi/qapi-visit-core.c
>>
>> Maybe we could include test/* here, so test code appears before
>> the implementation?
> 
> Hmmm, I'm not so sure about that. I certainly don't subscribe to TDD,
> when you first write up the test case, and then add implementation to
> satisfy the tests.
> 
> I don't have much experience / history with the tests subdir. Briefly
> reviewing what I've done there (git log --author=lersek), I think I
> would either keep the test patches entirely separate from the QEMU code
> patches (thus there wouldn't be a patch modifying *.c files both under
> and outside of tests/). Or else, I would actually prefer tests/*.c to
> come later in the same patch. The tests are frequently written to
> provide good coverage for the actual QEMU code, so seeing the QEMU code
> first seems preferable. (The corner cases to cover could be completely
> arbitrary, originating from quirks of POSIX functions, and so on.)
> 
> Thanks
> Laszlo
> 
>>> +*.c
>>> -- 
>>> 2.9.2
>>>
>>>
>>
> 

We also don't formally embrace TDD, as any tests that are checked in
_MUST_ pass. The patch order by definition then requires the feature
first, and then the tests.

Personally I like TDD, but I don't think it's appropriate to put tests
first in this instance, as we review with an eye for "Will this break
the tree right now?" and not "What are we about to add to QEMU?"

--js

  reply	other threads:[~2016-12-02 22:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-02 21:01 [Qemu-devel] [PATCH v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns Laszlo Ersek
2016-12-02 21:16 ` Eduardo Habkost
2016-12-02 21:54   ` Laszlo Ersek
2016-12-02 22:40     ` John Snow [this message]
2016-12-04 17:33 ` Michael S. Tsirkin
2016-12-05  0:30   ` Fam Zheng
2016-12-05  3:00     ` Michael S. Tsirkin
2016-12-05 10:24   ` Laszlo Ersek
2016-12-05 16:30     ` Eric Blake
2016-12-05 18:45     ` John Snow
2016-12-05 16:27   ` Eric Blake
2017-02-07 19:20 ` Eric Blake
2017-02-08  0:53   ` Laszlo Ersek
2017-06-27 18:51 ` Emilio G. Cota
2017-06-27 19:16   ` Michael S. Tsirkin
2017-06-27 19:39     ` Emilio G. Cota

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=840cbc6c-39f6-1b66-7ceb-f7caeae4fbf2@redhat.com \
    --to=jsnow@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=famz@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).