Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert the child handler blocksize change

["Laszlo Ersek" <lersek@redhat.com>]

On 10/21/20 03:33, Gao, Zhichao wrote:
> Hi Laszlo,
> 
> Apologize for the patch merge thing. That is my fault. Base on your comments, I update the commit message with revert info and part of my V1 commit message. And I am thinking there is no other doubt with the reverting. Sometimes the maintainers and reviewers my give the change comment and let the submitter not to send the patch again because the change is tiny.

Indeed this is very common, but it is always agreed upon beforehand.

> I treat this patch as that condition incorrectly. That's why I contact Ray to review the new commit message and help to push directly if he is OK with it. Sorry for missing you in the process.

Another problem is that the PR itself has to be created by the maintainer.

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-maintainer-process-for-the-edk-ii-project

As I understand it, the current process does not permit a contributor to
create a PR, and a maintainer to just set the "push" label on that PR.

Thanks
Laszlo


> 
> Thanks,
> Zhichao
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, October 20, 2020 5:53 PM
>> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
>> <zhichao.gao@intel.com>; glin@suse.com
>> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
>> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert the child
>> handler blocksize change
>>
>> Ray,
>>
>> On 10/19/20 07:56, Ni, Ray wrote:
>>> Zhichao,
>>> Can you please update the commit message to address Laszlo's comments?
>>
>> why did you merge <https://github.com/tianocore/edk2/pull/1033>?
>>
>> PR#1033 was originally submitted as a personal build for Zhichao. When it passed
>> CI (and was auto-closed), Zhichao should have posted the updated patch (with
>> the cleaned up commit message) as v2 to the mailing list, for the next round of
>> review.
>>
>> Instead, you reopened the (auto-closed) PR, added the push label, and the
>> mergify bot merged the patch. You merged a patch that was not reviewed on the
>> list; specifically you didn't give me any chance to re-check the commit message,
>> after I pointed out problems with it under the v1 thread. Of course, sometimes
>> we (participants in a patch review
>> thread) agree that the maintainer will perform some final (small) updates just
>> before merging the patch or series, but in this case, that has not happened -- no
>> specific wording was proposed or accepted in the thread, as far as I can see.
>>
>> Regardin the timeline: as of this writing, Zhichao opened PR#1033 eight hours
>> ago, and you made the mergify bot merge the (unreviewed) patch 4 hours ago.
>> That is, you just wanted to get rid of it as quickly as possible, without any regard
>> to the community (again -- I provided
>> *specific* feedback under v1).
>>
>> This urgency is especially appalling when contrasted with your and Eric's total
>> lack of feedback for Tom Lendacky's patch
>>
>>   [edk2-devel] [PATCH v2 1/1]
>>   UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure
>>
>>   https://edk2.groups.io/g/devel/message/65540
>>
>> for almost a *month*. I had to merge that patch yesterday out of desperation
>> and embarrassment for your behaviors, with only my R-b added. You forced me
>> to break the development process in order not to alienate a prolific contributor.
>> All one of you had to do was post an Acked-by.
>>
>> You can't make a contributor wait for this long, and you also can't sneak in
>> patches without public review (or at least public agreement about the final touch-
>> ups). If you can't do maintenance responsibly, for any reason, then *quit*; let
>> someone else become maintainer.
>>
>> This behavior is a hallmark of the edk2 project not having a healthy open source
>> community. Sad.
>>
>> Laszlo
>