From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45578) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aihPT-0001Kq-L2 for qemu-devel@nongnu.org; Wed, 23 Mar 2016 07:57:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aihPS-0002cC-M2 for qemu-devel@nongnu.org; Wed, 23 Mar 2016 07:57:11 -0400 Date: Wed, 23 Mar 2016 19:56:40 +0800 From: Peter Xu Message-ID: <20160323115640.GS28183@pxdev.xzpeter.org> References: <1458271654-23706-1-git-send-email-peterx@redhat.com> <1458271654-23706-2-git-send-email-peterx@redhat.com> <56F18FBC.3030909@redhat.com> <20160323030918.GE28183@pxdev.xzpeter.org> <87mvppbkbf.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87mvppbkbf.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v5 1/5] arm: qmp: add GICCapability struct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: wei@redhat.com, peter.maydell@linaro.org, drjones@redhat.com, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, qemu-arm@nongnu.org, abologna@redhat.com On Wed, Mar 23, 2016 at 10:44:04AM +0100, Markus Armbruster wrote: > Depends. > > The general rule is to keep separate things separate, and patches > self-contained. The narrow sense of self-contained is each patch > compiles and works. The wider sense is each patch makes sense to its > readers on its own. You can't always have a perfect score on the > latter, but you should try. > > Adding a definition without a user is generally not advised, because you > generally need to see the user to make sense of it. > > For a complex feature, adding its abstract interface before its concrete > implementation may help liberate interface review from implementation > details. > > Note that your interface consists of type GICCapability and command > query-gic-capabilities. You could add just the interface with a stub > implementation first, then flesh out the implementation. But I doubt > the thing is complex enough to justify that. Thanks for the thorough explaination on this. It's indeed not easy to figure out the best way every time for me... Now I do feel it strange to split the first patch alone from the 2nd one. Anyway, it's squashed into the 2nd patch in v6. -- peterx