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. > series > Acked-by: Laszlo Ersek <ler...@redhat.com> 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). -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42746): https://edk2.groups.io/g/devel/message/42746 Mute This Topic: https://groups.io/mt/32122467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
smime.p7s
Description: S/MIME cryptographic signature
