On 11/17/15 14:00, Zeng, Star wrote: > On 2015/11/17 20:24, Laszlo Ersek wrote: >> On 11/17/15 12:07, Star Zeng wrote: >>> Generally, this patch series are to upstream SerialDxe from >>> EmbeddedPkg to MdeModulePkg, >>> relatively, they are also to upstream SerialPortExtLib.h from >>> EmbeddedPkg to SerialPortLib.h in MdePkg. >>> >>> For your easy review, the forked code is at >>> g...@github.com:lzeng14/edk2.git branch SerialDxeV2. >>> >>> * New in V2: >>> According the suggestion from Laszlo Ersek <ler...@redhat.com>, >>> zero *Control first in SerialPortGetControl() for >>> EmulatorPkg/Omap35xxPkg/OvmfPkg updates. >> >> Please don't post a new version right after getting the initial >> comment(s) for the previous version -- it's usually prudent to give >> reviewers more time to review a posted version. >> >> In some exceptional cases the last version might be "completely broken", >> to the point where it makes no sense to continue (or even start) >> reviewing it. But in this specific case, the above change certainly >> doesn't warrant an immediate repost. >> >> I was still writing my second reply when you posted this version, so you >> simply couldn't have addressed *those* comments of mine in this version. >> Therefore I think I won't look at the series again before version 3. >> >> (And please give other reviewers more time so that v3 can also address >> their comments.) > > At the end of my working day, I just want to the people at the other > time zones can see the latest patch I have when they start their working > day.
Unfortunately, that idea doesn't scale. Other people (in different time zones) will see my review feedback *in addition* to your earlier patch series (that could now be somewhat obsolete, due to the most recent changes you've made), and will take my comments in account. Again, aside from absolute deal breakers in the previous version, there is no reason to post the next version of the series immediately. Consider, if you get reviews in a gradual, slow-ish fashion, like one or two patches reviewed per day, should you then post a version of the series *each day*? Absolutely not. You should post the next version if: - a reviewer has comprehensively reviewed the entire series, *or* - each patch received some feedback, possibly from different reviewers, *or* - a very intrusive change (that severely invalidates the last posted version) has turned out to be necessary, *or* - about a week passed since posting the last version, then you pinged people for feedback, but there is *still* no feedback. In other words: after 5-7 days of silence or so. *Or*, - someone explicitly asks for a full repost. I could have posted about several tens of versions of my SMM series (40-60 patches *each*, at various stages) until now, had I wanted people to see my "most current" status at the end of each rebase or working day. That doesn't scale at all; neither with regard to mailing list traffic, nor for reviewer capacity. Assume I'm at your patch #5 in my review, and you post a new version. Should I then abort my review and start anew, even if most of the remaining patches see no changes in the new version? Or should I complete the review, possibly in vain? Really, you have to give time to reviewers to "quiesce", and only post early only if there's an over-arching or otherwise intrusive change. In some development communities it is considered outright rude to post a series more frequently than once a week. (Think of the case when there are tens of people heavily working on separate features.) Here's an example: On 11/16/15 20:51, Laszlo Ersek wrote: > I have updated my local branch covering all the feedback I've received > for v4 thus far. For this patch in particular, no trace of > EFI_PEI_SMM_COMMUNICATION_PPI is left in the code. (I refreshed the > commit message, noting why we don't need EFI_PEI_SMM_COMMUNICATION_PPI > from under UefiCpuPkg.) > > I can post v5 tomorrow, or wait for more v4 feedback. I *could* post the next version right now, but there's no compelling reason to do so immediately. A bunch of the pending patches have not changed thus far, from v4 to v5 (v5 is "in progress"), so it actually helps reviewers if I let them continue reviewing v4. If you'd like to keep your most-current branch exposed to the world, you can do that on github (and post a short message about it, in response to the last version of the set). But, even in that case, the name of the frequently updated branch on github should not be versioned like v1, v2, v3, v4 -- the vN names should be in sync with mailing list postings. Instead, people usually call their frequently pushed / rebased branches "xxxx_queue" or "xxxx_wip" (as in "work in progress"). I'm glad that many developers have moved to git, and have even begun considering bisectability. Now we can start paying attention to the mailing list traffic. :) I do my best to provide feedback as quickly as I can, but as I said, you posted v2 while I was responding to your question / request for confirmation in the v1 thread. You didn't even await my answer with v2. (Jordan, BTW, do you intend to continue reviewing my stuff? Just askin'... O:-)) Thanks! Laszlo > > Thanks, > Star > >> >> Thanks >> Laszlo >> >>> >>> Star Zeng (12): >>> MdePkg SerialPortLib: Upstream GetControl/SetControl/SetAttributes >>> interfaces >>> PcAtChipsetPkg SerialIoLib: Add GetControl/SetControl/SetAttributes >>> implementation >>> MdeModulePkg BaseSerialPortLib16550: Add >>> GetControl/SetControl/SetAttributes implementation >>> MdeModulePkg: Upstream SerialDxe from EmbeddedPkg >>> EmulatorPkg: Use SerialDxe in MdeModulePkg instead of EmbeddedPkg >>> CorebootPayloadPkg: Use SerialDxe in MdeModulePkg >>> Omap35xxPkg SerialPortLib: Add GetControl/SetControl/SetAttributes >>> implementation >>> BeagleBoardPkg: Use SerialDxe in MdeModulePkg instead of EmbeddedPkg >>> ArmPlatformPkg: Use SerialDxe in MdeModulePkg instead of EmbeddedPkg >>> OvmfPkg XenConsoleSerialPortLib: Add >>> GetControl/SetControl/SetAttributes implementation >>> ArmVirtPkg: Use SerialDxe in MdeModulePkg instead of EmbeddedPkg >>> EmbeddedPkg: Remove SerialDxe and SerialPortExtLib libraries >>> >>> ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc | 3 +- >>> ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf | 3 +- >>> .../ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc | 3 +- >>> .../ArmVExpressPkg/ArmVExpress-CTA15-A7.fdf | 3 +- >>> .../ArmVExpressPkg/ArmVExpress-FVP-AArch64.dsc | 3 +- >>> .../ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf | 3 +- >>> .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc | 3 +- >>> .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.fdf | 3 +- >>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc | 1 - >>> .../PL011SerialPortLib/PL011SerialPortExtLib.c | 137 ------- >>> .../PL011SerialPortLib/PL011SerialPortExtLib.inf | 43 --- >>> .../PL011SerialPortLib/PL011SerialPortLib.c | 117 +++++- >>> ArmVirtPkg/ArmVirt.dsc.inc | 1 - >>> ArmVirtPkg/ArmVirtQemu.dsc | 2 +- >>> ArmVirtPkg/ArmVirtQemu.fdf | 2 +- >>> ArmVirtPkg/ArmVirtXen.dsc | 3 +- >>> ArmVirtPkg/ArmVirtXen.fdf | 3 +- >>> .../EarlyFdtPL011SerialPortLib.c | 82 ++++- >>> .../FdtPL011SerialPortLib/FdtPL011SerialPortLib.c | 99 ++++++ >>> BeagleBoardPkg/BeagleBoardPkg.dsc | 4 +- >>> BeagleBoardPkg/BeagleBoardPkg.fdf | 3 +- >>> CorebootPayloadPkg/CorebootPayloadPkg.fdf | 4 +- >>> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 11 +- >>> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 11 +- >>> .../Library/PlatformBdsLib/BdsPlatform.h | 5 +- >>> .../Library/PlatformHookLib/PlatformHookLib.c | 56 +++ >>> .../Library/PlatformHookLib/PlatformHookLib.inf | 38 ++ >>> .../Library/SerialPortLib/SerialPortLib.c | 316 >>> ----------------- > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
