From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 24 Jun 2019 14:49:28 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 60338C00732A; Mon, 24 Jun 2019 21:49:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-226.ams2.redhat.com [10.36.116.226]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2505A600CD; Mon, 24 Jun 2019 21:49:16 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 1/2] LegacyBios: set NumberBbsEntries to the size of BbsTable To: David Woodhouse , devel@edk2.groups.io, jljusten@gmail.com, afish@apple.com, Paolo Bonzini References: <9c9d53f5da9c3dec56cff0ff07c399e3034444a1.camel@infradead.org> <584f8af7979d40fd960122fc077fad02ecce09b1.camel@infradead.org> <29111f7d-0f4c-bba6-697d-c061ce1b6488@redhat.com> <3def810039016f9b48deff0526720136a996a7b3.camel@infradead.org> <3f9de0c1-afc3-1984-fe1c-4ea4021974ba@redhat.com> <06f6c0ecaa5214b901cadc244ec62ab53203871d.camel@infradead.org> From: "Laszlo Ersek" Message-ID: <9e74a47d-68fd-7f3c-0fd0-03874b896ddd@redhat.com> Date: Mon, 24 Jun 2019 23:49:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <06f6c0ecaa5214b901cadc244ec62ab53203871d.camel@infradead.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 24 Jun 2019 21:49:23 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 06/24/19 13:48, David Woodhouse wrote: > On Thu, 2019-06-20 at 22:35 +0200, Laszlo Ersek wrote: >> My past mistake doesn't excuse the current commit message ;) >> >> Anyway, I intentionally didn't ask for just repeating the commit msg >> title in the commit msg body. You could mention why the pre-patch >> expression (1 + 2 * MAX_IDE_CONTROLLER) is wrong, or wasteful. It's not >> obvious at all from just the patch itself (without looking at the larger >> context in the source file). You can save time for reviewers by giving >> hints in the commit message. And if you *can* save time for reviewers, >> you should. :) > > With specific requests for things that weren't already in the subject > line, I'm perfectly happy to add more. > > Better now? > >>>> I think your note on patch#2 is valuable too and should be captured in >>>> the commit message body. Please consider formulating it with a bit more >>>> neutral tone :) >>> >>> I would prefer to express that particular concern in 'diff -up' form >>> when actually fixing it :) >>> >> >> I'm a big fan of detailed commit messages. It's OK (and welcome) to >> describe current shortcomings even if you intend to fix them later. >> >> The commit message could also capture in one or two sentences what the >> patch does (locate handles with BlockIo instances, filter out some of >> them based on their DevicePath instances and other criteria, then for >> each handle, take the PCI B/D/F from the most specific PciIo instance on >> the device path). Anyway, I wouldn't like to obsess about this. > > Done. Thanks! >> series >> Acked-by: Laszlo Ersek > > Out of interest, why Acked-by: for these two vs. Reviewed-by: for the > first one? It's all under OvmfPkg now (which is made clearer in the > series I posted on Friday than it is in the subject line here). > For me anyway, R-b means a (reasonably) deep understanding & agreement. Acked-by means "looks okay, or at least I'm not worried about regressions in this area -- I'll let you do whatever you need to do". :) Thanks Laszlo