From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.138]) by mx.groups.io with SMTP id smtpd.web10.8059.1590126534843244718 for ; Thu, 21 May 2020 22:48:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=ZLsDK3wJ; spf=pass (domain: microsoft.com, ip: 40.107.244.138, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=G0etuW+xreedb0+pfj0XzI5LgOHtTWDmhIoDWuoW+g5hoSJYogLL5PuGdhQVPNt8tRS7FBELyms3+3lHqDtv1elNrowvNM+c1ohtGtT+UhSzcV6MpoLDTgDEDg9+YRixibbuXTor0EE0dvGRMCCqAU2jce1A+AP44JQq2HQJfb+nvzxig7AqOuvmLqJOHnUacSaIyhD/tE/lWzsEm0LB0lvk0Imn3LX2kPOSRoLtWMao+KO5LWj8DzM5/6YU6/5JBvcnsu/9Iu55bxwQyOO2d0VjXyXjlBxD7NuDLvg4Cr/CI3eb7MfCB2rX3dbaYGGnVe5KCpcJ8dLL1n+28HRBNA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WjHbw6gaT2rda/Az7/hV2jvIsCV3ifBiccm48Jzx6ms=; b=Q5Ob6WFQaRCt7Haz6CsavNbiMGAtQ9oxhEkRpg/Z2Oo2TgJOP2LyBsXpvaa6f4ErymcM2FzP2YKQGP9a8OgJAcoKHGUzKnAzNMJ4ysGiVcci7uro63lHusRcdY8yw7LYmNnMlTjrZnMhtUZsjuHlzQDfqKZQcAErCgVnOqXaD5ZMzvagqiguOd3hntSxRDOoyn4nADmrB/x0aUV8SyChMab0mfiGlv4d+5PqUmgU1+7DcvtKgq7yBhBawQIWJwHppXeXEl/ttDTclNGydMI9EgkwjeWlJP8n4y0goSct4VBpT3Jl7eAmd+OaqSUAooES+zFANyopeFm14zsx77BU/Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WjHbw6gaT2rda/Az7/hV2jvIsCV3ifBiccm48Jzx6ms=; b=ZLsDK3wJVXUkR0Cvxi4N0yL+uzridVe1MvCy2gqT27ZB8PyjOH86S7/haxuSLnGXpSG1WLFXkVVknXOCfi8OBOYDL290SbaX4R2Ewk8iw1bhuj6rJCB3x0B41ELRe+9MyZwvz6haaDtqCdb+cuA5iHwAuBsaRzOvG2PwYtFfDB0= Received: from CY4PR21MB0743.namprd21.prod.outlook.com (2603:10b6:903:b2::9) by CY4PR21MB0629.namprd21.prod.outlook.com (2603:10b6:903:12e::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3021.1; Fri, 22 May 2020 05:48:53 +0000 Received: from CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd]) by CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd%12]) with mapi id 15.20.3045.005; Fri, 22 May 2020 05:48:52 +0000 From: "Bret Barkelew" To: Andrew Fish , Laszlo Ersek CC: "devel@edk2.groups.io" , "spbrogan@outlook.com" , "rfc@edk2.groups.io" , "Desimone, Nathaniel L" , "Kinney, Michael D" , "Leif Lindholm (Nuvia address)" Subject: Re: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process Thread-Topic: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process Thread-Index: AdYtq0vCPqbiMowAQbSbICPKq6X1VgAUtBSAAAfspYAAA6RdAABR520AABxE5gAABccRDg== Date: Fri, 22 May 2020 05:48:52 +0000 Message-ID: References: , In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-05-22T05:45:19.2787852Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged authentication-results: apple.com; dkim=none (message not signed) header.d=none;apple.com; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [71.212.144.72] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 6980430a-5637-4270-794a-08d7fe13cf2d x-ms-traffictypediagnostic: CY4PR21MB0629: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:4303; x-forefront-prvs: 04111BAC64 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: rG3rdbIy/jY9VF9DPxZ9PHgyuv5QGWlTqfjNR85gDggKdFQptCR69JI6mIFu5d/xykNqi4xm+9qm9Kfpvsttcx76d3oZhrPeIy8StTC7zB2yKpExawptLTD+mBKFKkHfmfwdE/6xUY1PRVP0i02GQ3vcaGbWzsu+gdxqn0cxUY/ROQs9iNZYVIProIGA7s/kDlw5kZAOqnrsqROkLw+I1GsPL8JbBgKOZSE/bBdfSTSqntOIryIsiqixQOofB1t7FBA7tmucA4pITckH5Uu2HO+t4DPH4wiHEKTGnXo5K8iLzuhOH0Dwl3Sakd3CKdOoUFghhrpianm301/gyCmRdA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY4PR21MB0743.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(39860400002)(396003)(136003)(366004)(346002)(376002)(66946007)(8990500004)(110136005)(316002)(76116006)(82960400001)(33656002)(82950400001)(66556008)(478600001)(5660300002)(64756008)(66446008)(66476007)(30864003)(91956017)(54906003)(2906002)(6506007)(8936002)(7696005)(8676002)(186003)(53546011)(71200400001)(55016002)(9686003)(26005)(52536014)(10290500003)(4326008)(86362001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: m9SSbzucHVI2WjVNflytuNpsGJUZ9jQbe+kazQQ9oaWMlql/4IEWOcx0Owi3DrxE1I/SMF+HMSjnXg/PcpmePHgPNO9X1+2YlLM1tBkfCKWKTQv2VynmxdA/qNPfLiXuFVwC/ncYvMIbriMyr4Bcbd8Jc11DJuz/3T1PMHAsBKmI0t6Sq4/u8Zo3kywv2kiW0xsJIy3zh0sGAabIEAGjrSaROEDDfLaqNiqXe9mSxHKM2PSH3rXSiniYYffNAIno4MjBZ+cYRUNH3hoa85h0dI3+S3rRYp1qITJAaLbG9nraDoO6yEUhIOzDgHWWlOrCr51HJCJgCXmGuQNqbI8/m5+tRK89WokwlQfPSZoVe4amRxF85GBXKrqm7WMybk9oypIUD8kj2Aam3CLMG+phLseA17UzhSHlSexZOrM3zVCGlU5u2cCt4Wgf4D13+uK7BdtytF6kxemL85X23Uupc8NKefLtosRo0wZQmSL6OEI= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6980430a-5637-4270-794a-08d7fe13cf2d X-MS-Exchange-CrossTenant-originalarrivaltime: 22 May 2020 05:48:52.8322 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 69V+DH9AQl29piEHf6LBEXcc4M2K4B3p44toIic/w0dSSD+mVVYa8K9sxUG3U2Jdp1yD2006kFKSFoj/ZpfQOg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR21MB0629 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CY4PR21MB074333F2F05273A1D5C88B0AEFB40CY4PR21MB0743namp_" --_000_CY4PR21MB074333F2F05273A1D5C88B0AEFB40CY4PR21MB0743namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable =93But we do have tools that insert the Bugzilla number in all the commits = of the series, assist with the squash, etc.=94 Are these internal-only, or are they something we could evaluate when we mo= ve to the PR process? If not, are they based on anything we could leverage? I believe that the plan is to stick with Bugzilla for the immediate future = rather than use GitHub Issues (which is probably for the best for now, give= n the span across repos and access levels), so any tooling to tie that toge= ther would be interesting to evaluate. In Mu we have a similar problem of keeping track of what features/bugs have= already been upstreamed and when can they be dropped during an upstream in= tegration, so that=92s the more personal interest I have in such automation= . Thanks! - Bret From: Andrew Fish Sent: Thursday, May 21, 2020 8:00 PM To: Laszlo Ersek Cc: devel@edk2.groups.io; spbrogan@outlook.com= ; rfc@edk2.groups.io; Desimone, Nathaniel L; Bret Barke= lew; Kinney, Michael D; Leif Lindholm (Nuvia address) Subject: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based C= ode Review Process > On May 21, 2020, at 6:30 AM, Laszlo Ersek wrote: > > On 05/20/20 00:25, Sean wrote: >> On 5/19/2020 1:41 PM, Laszlo Ersek wrote: > >>> Your proposal to "don't exclude squash merge workflows" is a trap. If >>> we tolerate that option -- which is obviously the sloppy, and hence >>> more convenient, option for some maintainers and some contributors, >>> to the detriment of the git history --, then almost every core >>> maintainer will use it as frequently as they can. In the long term, >>> that will hurt many consumers of the core code. It will limit the >>> ability of people not regularly dealing with a particular core module >>> to file a fine-grained bug report for that module, maybe even propose >>> a fix. From the regression analyst's side, if the bug report starts >>> with "I have a bisection log", that's already a good day. And your >>> proposal would destroy that option, because maintainers and people in >>> general are irrepairably lazy and undisciplined. We cannot post a >>> community member shoulder-by-shoulder with every core package >>> reviewer/maintainer to prevent the latter from approving a >>> squash-on-merge, out of pure laziness. I'm 100% sure the "option" to >>> squash-on-merge would *immediately* be abused for a lot more than >>> just "typo fixes". There isn't enough manpower to watch the watchers, >>> so "no squash-on-merge" needs to be a general rule. >> >> >> I have trouble with this line of thinking. The maintainers are and >> should be considered the representatives of this code base. They >> have a vested interest to enable this repository to work for them. If >> they really are viewed as "sloppy" or "lazy" then we are destined to >> fail anyway. > > You put it very well. "They have a vested interest to enable this > repository to work for them." Key part being "*for them*". > > Core maintainers are responsible for making this repository work for a > lot larger camp than just themselves. Even if squash-on-merge satisfied > the requirements that core maintainers presented, squash-on-merge would > still hurt the larger community that depends on those packages. > > The core-consumer community may not necessarily participate in the > day-to-day maintenance of the core packages, but they do report bugs and > even contributes bugfixes / occasional features, when their particular > use cases require those actions. > > And squash-on-merge hurts those activities, down the road, because the > git history is instrumental to analyzing and learning the code base. > > For example, the question "why do we call this function here?" > immediately leads to running "git blame" (possibly a series of git-blame > commands, to navigate past code movements and such). In the end > git-blame leads to a particular commit, and that commit is supposed to > answer the question. If the commit is huge (e.g. a squash of an entire > feature), then the question is not answered, and git-blame has been > rendered useless. > > >> Nothing in my statement of "don't exclude squash merge workflow" >> requested that we allow a PR to be squashed into a single commit that >> you believe should be a patch series. > > If the button is there, maintainers will click it even in cases when > they shouldn't, and I won't be able to catch them. The result will not > necessarily hurt the maintainer (not at once, anyway), but it will harm > others that investigate the git history afterwards -- possibly years > later. > > I can't watch all CoreFoobarPkg pull requests on github to prevent this. > On the other hand, I can, and do, monitor the edk2-devel list for > seriously mis-organized patch sets, especially for core packages where > I've formed an "I had better watch out for this core package" > impression. > > I have made requests under core patch sets where I was mostly unfamiliar > with the technical subject *for the time being*, asking just for > improvements to the granularity of the series. Knowing the improved > granularity might very well help me *in the future*. > > > The mailing list equivalent of "squash-on-merge" would be the following: > > - contributor posts v1 with patches 1/5 .. 5/5 (for example), > > - reviewer requests updates A, B, and C, > > - contributor posts (in response to the v1 blurb, i.e. 0/5) further > patches 6/8, 7/8, 8/8 > > - reviewer checks the new patches and approves them, functionally, > > - maintainer says "OK let me merge this", > > - maintainer applies the patches (all 8 of them) from the list, on a > local branch, > > - maintainer runs a git rebase squashing the whole thing into a single > patch, > > - maintainer does *not* review the result, > > - maintainer opens a PR with the resultant single patch, > > - CI passes, > > - the patch is merged. > > > With the list-based process, the disaster in the last step is mitigated > in at least three spots: > > - All subscribers have a reasonably good chance to notice and intervene > when the incremental fixups 6/8, 7/8, 8/8 are posted as followups to > the v1 blurb, clearly with an intent to squash. > > - Because the maintainer has to do *extra work* for the squashing, the > natural laziness of the maintainer works *against* the disaster. Thus > he or she will likely not perform the local rebase & squash. Instead > they will ask the contributor to perform a *fine-grained* squash (i.e. > squash each fixup into the one original patch where the fixup > belongs), and to submit a v2 series. > > - If someone interested in the git history catches (after the fact) that > the maintainer merged a significantly different patch set from what > had been posted and reviewed, they can raise a stern complaint on the > list, and next time the maintainer will now better. > > (This is not a theoretical option; I low-key follow both the list > traffic and the new commits in the git history (whenever I pull). In the > past I had reported several patch application violations (mismanaged > feedback tags, intrusive updates post-review, etc). Nowadays it's gotten > quite OK, thankfully, and I'm terrified of losing those improvements.) > > > If we just plaster a huge squash-on-merge button or checkbox over the > web UI, it *will* be abused (maintainer laziness will work *towards* the > disaster), with only a microscopic chance for me to prevent the abuse. > > It's not that "I believe" that this or that *particular* series should > not be squashed. "Not squashing" is not the exception but the rule. The > *default* approach is that the submitter incorporates incremental fixes > into the series at the right stages, they maintain a proper series > structure over the iterations, and they propose revised versions of the > series in full. Squashing is the exception; for example one reason is, > "if you separate these changes from each other, then the tree will not > build in the middle; they belong together, please squash them, and > resubmit for review". > > >> I do think those rules will need to be defined but that is needed >> today anyway. > > Rules are only as good as their enforcement is. > In my work world we require code review by a manager and that is the de fac= to enforcement mechanism. Basically there is always an owner to make sure t= he process was followed :) Also in our world the squash is a developer choice. But we do have tools th= at insert the Bugzilla number in all the commits of the series, assist with= the squash, etc. > The question is not how nice it is to use squash-on-merge in the > minuscule set of situations when it might be justified; the question is > how difficult it would be to prevent the inevitable abuses. > > The list lets me advocate for proper git history hygiene reasonably > efficiently (although I still miss a bunch of warts due to lack of > capacity). With the squash-on-merge button or checkbox, the flood gates > would fly open. I won't stand for that (not as a steward anyway). > > I think our world views differ fundamentally. I value the git history > *way* above my own comfort, and everyone else's (accounting for both > contributor and day-to-day maintainer roles). I guess you prefer the > reciprocal of that ratio. > I'd also point out that the processes you chose kind of defines your quanta= of work. It is likely you would be willing to tackle a really big change a= s a large patch set, that you would likely break up into multiple PRs in a = squash on commit world. In a squash on commit world you also might break a = Bugzilla (BZ) up into dependent BZs, a tree of BZs. That might sound crazy,= but when you work on a bigger project and there are BZs for EFI, T2, macOS= , the Installer, and the RecoveryOS for a customer visible feature this tre= e of BZ might be familiar and make sense to you. But I think the real argument for consistency is we have a rich git history= that has value. We have made resource tradeoffs to have that rich git hist= ory so to me it makes the most sense, for these project, to try to preserve= our past investment in git history. Thanks, Andrew Fish > Thanks, > Laszlo > --_000_CY4PR21MB074333F2F05273A1D5C88B0AEFB40CY4PR21MB0743namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

=93But we do have tools that insert the Bugzilla num= ber in all the commits of the series, assist with the squash, etc.=94<= /o:p>

 

Are these internal-only, or are they something we co= uld evaluate when we move to the PR process? If not, are they based on anyt= hing we could leverage?

 

I believe that the plan is to stick with Bugzilla fo= r the immediate future rather than use GitHub Issues (which is probably for= the best for now, given the span across repos and access levels), so any t= ooling to tie that together would be interesting to evaluate.

 

<Tangent>

In Mu we have a similar problem of keeping track of = what features/bugs have already been upstreamed and when can they be droppe= d during an upstream integration, so that=92s the more personal interest I = have in such automation.

 

Thanks!

 

- Bret

 

From: Andrew Fish
Sent: Thursday, May 21, 2020 8:00 PM
To: Laszlo Ersek
Cc: devel@edk2.groups.io= ; spbrogan@outlook.com; rfc@edk2.gr= oups.io; Desimone, Nathaniel L= ; Bret Barkelew; Kinney, Mi= chael D; Leif Lindholm (Nuvia address)
Subject: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request = based Code Review Process

 



> On May 21, 2020, at 6:30 AM, Laszlo Ersek <lersek@redhat.com> wr= ote:
>
> On 05/20/20 00:25, Sean wrote:
>> On 5/19/2020 1:41 PM, Laszlo Ersek wrote:
>
>>> Your proposal to "don't exclude squash merge workflows&qu= ot; is a trap. If
>>> we tolerate that option -- which is obviously the sloppy, and = hence
>>> more convenient, option for some maintainers and some contribu= tors,
>>> to the detriment of the git history --, then almost every core=
>>> maintainer will use it as frequently as they can. In the long = term,
>>> that will hurt many consumers of the core code. It will limit = the
>>> ability of people not regularly dealing with a particular core= module
>>> to file a fine-grained bug report for that module, maybe even = propose
>>> a fix. From the regression analyst's side, if the bug report s= tarts
>>> with "I have a bisection log", that's already a good= day. And your
>>> proposal would destroy that option, because maintainers and pe= ople in
>>> general are irrepairably lazy and undisciplined. We cannot pos= t a
>>> community member shoulder-by-shoulder with every core package<= br> >>> reviewer/maintainer to prevent the latter from approving a
>>> squash-on-merge, out of pure laziness. I'm 100% sure the "= ;option" to
>>> squash-on-merge would *immediately* be abused for a lot more t= han
>>> just "typo fixes". There isn't enough manpower to wa= tch the watchers,
>>> so "no squash-on-merge" needs to be a general rule.<= br> >>
>>
>> I have trouble with this line of thinking. The maintainers are and=
>> should be considered the representatives of this code base. &= nbsp; They
>> have a vested interest to enable this repository to work for them.=   If
>> they really are viewed as "sloppy" or "lazy" t= hen we are destined to
>> fail anyway.
>
> You put it very well. "They have a vested interest to enable this=
> repository to work for them." Key part being "*for them*&quo= t;.
>
> Core maintainers are responsible for making this repository work for a=
> lot larger camp than just themselves. Even if squash-on-merge satisfie= d
> the requirements that core maintainers presented, squash-on-merge woul= d
> still hurt the larger community that depends on those packages.
>
> The core-consumer community may not necessarily participate in the
> day-to-day maintenance of the core packages, but they do report bugs a= nd
> even contributes bugfixes / occasional features, when their particular=
> use cases require those actions.
>
> And squash-on-merge hurts those activities, down the road, because the=
> git history is instrumental to analyzing and learning the code base. >
> For example, the question "why do we call this function here?&quo= t;
> immediately leads to running "git blame" (possibly a series = of git-blame
> commands, to navigate past code movements and such). In the end
> git-blame leads to a particular commit, and that commit is supposed to=
> answer the question. If the commit is huge (e.g. a squash of an entire=
> feature), then the question is not answered, and git-blame has been > rendered useless.
>
>
>> Nothing in my statement of "don't exclude squash merge workfl= ow"
>> requested that we allow a PR to be squashed into a single commit t= hat
>> you believe should be a patch series.
>
> If the button is there, maintainers will click it even in cases when > they shouldn't, and I won't be able to catch them. The result will not=
> necessarily hurt the maintainer (not at once, anyway), but it will har= m
> others that investigate the git history afterwards -- possibly years > later.
>
> I can't watch all CoreFoobarPkg pull requests on github to prevent thi= s.
> On the other hand, I can, and do, monitor the edk2-devel list for
> seriously mis-organized patch sets, especially for core packages where=
> I've formed an "I had better watch out for this core package"= ;
> impression.
>
> I have made requests under core patch sets where I was mostly unfamili= ar
> with the technical subject *for the time being*, asking just for
> improvements to the granularity of the series. Knowing the improved > granularity might very well help me *in the future*.
>
>
> The mailing list equivalent of "squash-on-merge" would be th= e following:
>
> - contributor posts v1 with patches 1/5 .. 5/5 (for example),
>
> - reviewer requests updates A, B, and C,
>
> - contributor posts (in response to the v1 blurb, i.e. 0/5) further >  patches 6/8, 7/8, 8/8
>
> - reviewer checks the new patches and approves them, functionally,
>
> - maintainer says "OK let me merge this",
>
> - maintainer applies the patches (all 8 of them) from the list, on a >  local branch,
>
> - maintainer runs a git rebase squashing the whole thing into a single=
>  patch,
>
> - maintainer does *not* review the result,
>
> - maintainer opens a PR with the resultant single patch,
>
> - CI passes,
>
> - the patch is merged.
>
>
> With the list-based process, the disaster in the last step is mitigate= d
> in at least three spots:
>
> - All subscribers have a reasonably good chance to notice and interven= e
>  when the incremental fixups 6/8, 7/8, 8/8 are posted as followup= s to
>  the v1 blurb, clearly with an intent to squash.
>
> - Because the maintainer has to do *extra work* for the squashing, the=
>  natural laziness of the maintainer works *against* the disaster.= Thus
>  he or she will likely not perform the local rebase & squash.= Instead
>  they will ask the contributor to perform a *fine-grained* squash= (i.e.
>  squash each fixup into the one original patch where the fixup >  belongs), and to submit a v2 series.
>
> - If someone interested in the git history catches (after the fact) th= at
>  the maintainer merged a significantly different patch set from w= hat
>  had been posted and reviewed, they can raise a stern complaint o= n the
>  list, and next time the maintainer will now better.
>
> (This is not a theoretical option; I low-key follow both the list
> traffic and the new commits in the git history (whenever I pull). In t= he
> past I had reported several patch application violations (mismanaged > feedback tags, intrusive updates post-review, etc). Nowadays it's gott= en
> quite OK, thankfully, and I'm terrified of losing those improvements.)=
>
>
> If we just plaster a huge squash-on-merge button or checkbox over the<= br> > web UI, it *will* be abused (maintainer laziness will work *towards* t= he
> disaster), with only a microscopic chance for me to prevent the abuse.=
>
> It's not that "I believe" that this or that *particular* ser= ies should
> not be squashed. "Not squashing" is not the exception but th= e rule. The
> *default* approach is that the submitter incorporates incremental fixe= s
> into the series at the right stages, they maintain a proper series
> structure over the iterations, and they propose revised versions of th= e
> series in full. Squashing is the exception; for example one reason is,=
> "if you separate these changes from each other, then the tree wil= l not
> build in the middle; they belong together, please squash them, and
> resubmit for review".
>
>
>> I do think those rules will need to be defined but that is needed<= br> >> today anyway.
>
> Rules are only as good as their enforcement is.
>

In my work world we require code review by a manager and that is the de fac= to enforcement mechanism. Basically there is always an owner to make sure t= he process was followed :)

Also in our world the squash is a developer choice. But we do have tools th= at insert the Bugzilla number in all the commits of the series, assist with= the squash, etc.

> The question is not how nice it is to use squash-on-merge in the
> minuscule set of situations when it might be justified; the question i= s
> how difficult it would be to  prevent the inevitable abuses.
>
> The list lets me advocate for proper git history hygiene reasonably > efficiently (although I still miss a bunch of warts due to lack of
> capacity). With the squash-on-merge button or checkbox, the flood gate= s
> would fly open. I won't stand for that (not as a steward anyway).
>
> I think our world views differ fundamentally. I value the git history<= br> > *way* above my own comfort, and everyone else's (accounting for both > contributor and day-to-day maintainer roles). I guess you prefer the > reciprocal of that ratio.
>

I'd also point out that the processes you chose kind of defines your quanta= of work. It is likely you would be willing to tackle a really big change a= s a large patch set, that you would likely break up into multiple PRs in a = squash on commit world. In a squash on commit world you also might break a Bugzilla (BZ) up into dependent BZs= , a tree of BZs. That might sound crazy, but when you work on a bigger proj= ect and there are BZs for EFI, T2, macOS, the Installer, and the RecoveryOS= for a customer visible feature this tree of BZ might be familiar and make sense  to you.

But I think the real argument for consistency is we have a rich git history= that has value. We have made resource tradeoffs to have that rich git hist= ory so to me it makes the most sense, for these project, to try to preserve= our past investment in git history.

Thanks,

Andrew Fish

> Thanks,
> Laszlo
>

 

--_000_CY4PR21MB074333F2F05273A1D5C88B0AEFB40CY4PR21MB0743namp_--