From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (NAM11-DM6-obe.outbound.protection.outlook.com [40.92.19.76]) by mx.groups.io with SMTP id smtpd.web12.848.1668452940194669451 for ; Mon, 14 Nov 2022 11:09:00 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@outlook.com header.s=selector1 header.b=NvXTsMKP; spf=pass (domain: outlook.com, ip: 40.92.19.76, mailfrom: spbrogan@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YIGLoSAUyPafLisz7udFF4UevPvFThreQDdlrkCcaZwkCgD9Q2pO7Id7bkqIOQsnnoxpP6UniKtrhALfAqa9WPOQdADYFrUq4YsHLkNtaJQwZ+a2L25R7vq3iiFzdLBa8cCyGZxIS6EC3fNXXBnqBQ1iAr6FgujWbjIC+2YB9kkkKLo9jAOncGyHoB9bBdhTqj2x3CL49S406FNbQR2PYzY7UZnQEq/NjH9Jog/YcL55cJx/YQV04I59tW1aKvIX+7BHT54nDJJivMvJpdqjvpbqiQLL8E31Jy39RCHG+cAivVjQc64nzo5yGDxXRnM4fAQAU0GlVGBUwrubJA3TXg== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=CyaNlCODTC7VlQrv6WMlPemF8cYmhvn4iQl+NEZDlSs=; b=Gtwn0Yn2CjWcsq44o5sXSwNrn5S1rnnA4nguquj9FGPl0xyzL2tPikOuuZV+ndK9BmPIUSz9P2Pli4TAC7Hrl6n8r94HBCq0gFzxZ4Ot81UyWYTA7Y3daFNWSWxrtaq730YlurW2iHXvpE1+Ygdx20Z37Er+mjP/3g2hYSoaLVoUBu3Bz16E5LzI4OTxmxvJ27NzTOsCagIjcbzsKYezmc2yGCnMiwFvThd+eL2x8d8vDOzExqrYM2uQtCG+tqM+OGxvy4AUnFlFCdDAJN/8yc+WjaxHgSBtm10tmhliWoVR1Du5asTnavq8Ugd36rv9H3RHn3WFbBTUzG4W251pTw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CyaNlCODTC7VlQrv6WMlPemF8cYmhvn4iQl+NEZDlSs=; b=NvXTsMKPIB33pxf0Y1e3EFuc9XSbShzITec+d0GZughJrja4nerY1KLWeb+PK+9sB3m0FKi0zIrJ/qjPIY71s87cA6Mdd0AQRJmf1Mny8yciG6MCucJiDGHSAGdK8IUKitBt5UqEBaPzM7Dz8QbFMiRguiSSZCmhqXp5UQyq8RtFA6qP+r56JLanQ6ZucyU/RsTsY7hHTZxTDKZ9NhUj7uCP6FzeovFqGOaXGClx/IXcNPwakvx82Mi06E9KwOMXlGon9X6jE4OQB+SUHuikzE0QXcvlm5bEMwADtVU4f6wGG4ll6w7NCAMV5yZ/oZCTwDQs5e2YTHcFLejvFgrU2Q== Received: from BY3PR19MB4900.namprd19.prod.outlook.com (2603:10b6:a03:354::11) by PH7PR19MB6636.namprd19.prod.outlook.com (2603:10b6:510:1ab::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5791.25; Mon, 14 Nov 2022 19:08:58 +0000 Received: from BY3PR19MB4900.namprd19.prod.outlook.com ([fe80::bd24:dbff:ccbc:aba]) by BY3PR19MB4900.namprd19.prod.outlook.com ([fe80::bd24:dbff:ccbc:aba%9]) with mapi id 15.20.5813.017; Mon, 14 Nov 2022 19:08:58 +0000 Message-ID: Date: Mon, 14 Nov 2022 11:08:56 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments To: devel@edk2.groups.io, mikuback@linux.microsoft.com, "Kinney, Michael D" , "abner.chang@amd.com" , Laszlo Ersek , "Kubacki, Michael" References: <922996c2-e60a-80ec-6d4a-8b2e5a639c9b@redhat.com> <25893.1668303336748921539@groups.io> <17278424C4A5D78F.32003@groups.io> <7ed9dd39-71c2-f175-9ee9-f7f2f7b89e23@linux.microsoft.com> From: "Sean" In-Reply-To: <7ed9dd39-71c2-f175-9ee9-f7f2f7b89e23@linux.microsoft.com> X-TMN: [xdGxxI2q/8lkt0x4eQC8/+YZDxA55mby] X-ClientProxiedBy: MW4PR03CA0128.namprd03.prod.outlook.com (2603:10b6:303:8c::13) To BY3PR19MB4900.namprd19.prod.outlook.com (2603:10b6:a03:354::11) Return-Path: spbrogan@outlook.com X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BY3PR19MB4900:EE_|PH7PR19MB6636:EE_ X-MS-Office365-Filtering-Correlation-Id: 99b02b66-80c0-48ba-a2f6-08dac673aeb1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: xIC4wrnoEeai8x5/QDVwfo4bv1T3lxh/SbNqDSKaKjsb5dq7zm/ve8bXlxL5/s9Ie6PVsf8VKmdp8jC5+8SuQN4dJBBPtYcrKrPp/+cxgID4v74iAmAEvj+I16x1ykY7TcFV8duNh77JPw0oCbAjJ38pNRF2qeu5JIh6/W5Ei5LMezagUULsE2KYewN53jcvAYpSvS1wzlHwqegobhNUxJA+Wm978G2cXS+eyouvtjn80Eny/djE6dd62cVoiHl0KUXOAa5Vytwrl1FDK4jlMRSyehJIdGOOrNCJHHACW2kCZqCDrHr/PIB21jlIWSWLuVegp+4YGTjoguiKGkozcxFlHuADhqpfRaOEY9EQzeSkRP+JfV0A/96EfqJO21/mw09W/42goP7MMMGyZ3Qp/7yl/fTrwjv6e+UhC1Za1NwFbp5te5m9yRWKP+TkcS381N0t9W76MTlEp1iFC76XElhYJAa0x3oKD9MCZCS1SWL5WDklenwk6Ecd7GF9aCYq4xWeAbfB0xDfqgp6N9wuHyPdelFU9q4VlhvrbB/wmgC6zXAMUsTIReH/XFup/a4ufugGtjAm0ApV7KlBRCeaLOSopcmFQQKah+O1PUDxDEq8t978HJ5aaOhhn+McCp9OWwcWbj6dHp81YMAnzSvsd9otrjkDQmOC5eeRMkKRRkT6lUlc8UUMwXVt1VT4EUKYsbrh+oe+AN7IQZfvK/uYTCCmwqAqPbuJ1hwVpuhx0sc= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?d3p+kyeu5b341BVDKFHl3oiQ5fkOmF8qL1VzlzB0hdhXQMyxVpyq6ropFJeG?= =?us-ascii?Q?+Rtm1XWQnOsolCiIapPDIvNVSaTmt+AcuH5bl+miZXe7HKUda0LvyN8G/uK0?= =?us-ascii?Q?tq4FoquqU60fOnOpT4ubbfcWkmZx+A6WGdudkDKjcTNlJOvkNia63TN2ZrAd?= =?us-ascii?Q?HWdZMRE3nZYWt6TnNqI/cdOtAro183lsbvMHgfsMr9a+f8PXAIL3knMsov6N?= =?us-ascii?Q?f8Vg2dtcg5DLoYNQ6+nMmQ7Xz8ZO0DgxAmvXdcMYU6FYW87YzUMsOFVmyAy8?= =?us-ascii?Q?rbEoD22JEqyDWRUR4eaqRHvJsf6AB9aq4SUw/arcSNeowrBDdGXr+BHHoUH5?= =?us-ascii?Q?nspTt3ti3Tt2b7aaKer8nG1WzwZiCHzQbNRVVIg9BGTG8kIKCgewIXhPhEfH?= =?us-ascii?Q?C2jh7TJiDxTrFpMBNiIYFwrrF9rGle1YAh1TM+9Tjq6Dvn2MyYPg7Ywsx7JN?= =?us-ascii?Q?gTmF5wNojJCa6ItgkIX/6MhEPrh8+VE/bUN/NwTRjkuYA0UkaMwuh1Gz8LYx?= =?us-ascii?Q?LLe19jfdYRdkwf8q7JbpHY1OIbxl6zn4lhVPLbMOnn9uerlPhdxRPLRo9g8q?= =?us-ascii?Q?VaBU7cqEI9bFvlUhRPLW+z5D/6atwuqFEL7E9AiTEpgcCV1BEi9ihJgXKH4W?= =?us-ascii?Q?L+Laf+UEPVeE6779a/5IksnY90QCV3p4UQ74ieXztIj3pVPn8xU7BltaIW/p?= =?us-ascii?Q?mZCTAnkjJym8AbF5SrSO7ux/eSEP3NFYVblhKb2zlW1raQhp8SB0tmVY8JmD?= =?us-ascii?Q?R1YHKk0wlKlKAZh4YWq34QEqIPN2J47Wrqvj4Aw6RtYYyg083piL70njlcNj?= =?us-ascii?Q?LbzztXM6IwcQj8KKGJaGu/+WaGsVHjaXj5GqvIwWXd9I8UDRQLCn5D69MGNh?= =?us-ascii?Q?IvRjLR+QDFECCZxyj24sdz9UVcO/z4rLbPg/9CfBUGf1y5KLNlso8YK5MKQG?= =?us-ascii?Q?RPtHNpa31IC32t7UsBxdrVu/645kdSReQp5VLyLeN+qVdu6Y7VxqxzBYKKRU?= =?us-ascii?Q?T9lw3ILEITUrHk3Nk+P7svJEeXfIMdfzrWOrN+cHzRSFeHVu9kIkSW9WORZK?= =?us-ascii?Q?khGfAFIgwt9wPC3koWO5VZZxnS1xlW38xjJxJZ48l5mqMbsaVD04WJWa1YIP?= =?us-ascii?Q?A+/aoVuuoQRumUKF3GhOTQPuUYRwMK2LmLUoLNot8R3ZpL1swBNrbQUUd39Z?= =?us-ascii?Q?+0Cv4JxYg8s+yvOpIetTVKQzLlTqm15NaoJ7RZH+zKQeTAr42U3S+C1tuzoU?= =?us-ascii?Q?XrfR3gvEJNlDbvpZgLdPS/aEWvSq+SfgdHpIUfCFpg=3D=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 99b02b66-80c0-48ba-a2f6-08dac673aeb1 X-MS-Exchange-CrossTenant-AuthSource: BY3PR19MB4900.namprd19.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Nov 2022 19:08:58.3730 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR19MB6636 Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable If you look through the bugs you will see there is some debate.=20 Personally, I think there is never a single right answer on style and=20 there will always be differing opinions.=C2=A0 The underlying goal that mos= t=20 agree on is providing consistency for a project and to make it easy for=20 someone to contribute code that aligns with that consistent standard.=C2=A0= =C2=A0=20 Since 2017, a lot has changed but the biggest thing is we have a=20 relatively easy, well documented, industry standard tool that supports=20 style/formatting of edk2. These tools can be run locally by those=20 contributing and are run by the CI system on pull request to enforce=20 this consistency.=C2=A0 We have already reformatted the entire code base of= =20 edk2 to align with that and although i am not opposed to auto=20 reformatting in the future for a great reason I don't see anything is=20 these proposed changes that would justify that. Thanks Sean On 11/14/2022 10:49 AM, Michael Kubacki wrote: > I saw those, but they're from over 5 years ago and the community has=20 > changed since then. > > This is an impactful change. It will impact every line of code written. > > I'm not suggesting this go to the Tools & CI Meeting because of=20 > uncrustify. I'm not concerned with any impact to uncrustify. > > This came to my attention because I was added to thread. I would like=20 > to suggest that we raise more awareness about this change in a meeting=20 > with members of the community to capture additional feedback. > > Also, the logistics (uncrustify aside) about how to execute this=20 > change are unclear (as written in this thread and the BZ) and it would=20 > be easier/faster to discuss in a community call. > > Is that possible? > > Thanks, > Michael > > On 11/14/2022 1:25 PM, Kinney, Michael D wrote: >> Hi Michael, >> >> Yes.=C2=A0 They have been discussed.=C2=A0 There were patches and discus= sion on=20 >> mailing >> lists back in 2017.=C2=A0 Long before enabling uncrustify. I provided=20 >> links to >> these conversations in the following email about a week ago. >> >> https://edk2.groups.io/g/devel/message/96038 >> >> >> Mike >> >>> -----Original Message----- >>> From: devel@edk2.groups.io On Behalf Of=20 >>> Michael Kubacki >>> Sent: Monday, November 14, 2022 10:05 AM >>> To: devel@edk2.groups.io; Kinney, Michael D=20 >>> ; abner.chang@amd.com; Laszlo Ersek=20 >>> ; >>> Kubacki, Michael >>> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH=20 >>> 2/2] Source Files / Spacing / Multi-line func. calls: allow >>> condensed arguments >>> >>> Have these changes been discussed in a community forum? Do you think we >>> could talk about it in the Tianocore Tools & CI Meeting today? >>> >>> Thanks, >>> Michael >>> >>> On 11/14/2022 12:37 PM, Michael Kubacki wrote: >>>> I'm catching up on this thread so let me know if I miss something. >>>> >>>> Uncrustify can perform conversions/enforcements like adjusting code=20 >>>> to < >>>> 80 columns. >>>> >>>> The edk2 uncrustify configuration file is here and you will see that I >>>> commented column width enforcement: >>>> >>>> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/Uncrustif= yCheck/uncrustify.cfg#L19.=20 >>>> >>>> >>>> >>>> Uncrustify aside, column width is particularly difficult to adjust >>>> consistently. Both humans and tools have to make many (constant) >>>> situational decisions based on code structure. >>>> >>>> However, it should be possible. I've taken the current uncrustify >>>> configuration file, made minimal changes to restrict code to 80=20 >>>> columns, >>>> and published the results based on the current edk2 code tree here: >>>> >>>> https://github.com/makubacki/edk2/tree/uncrustify_80_column_change >>>> >>>> You can see the I ran uncrustify 3 times to reach a mostly steady=20 >>>> state. >>>> The issue after the second run is that uncrustify is confused about=20 >>>> what >>>> to do with single macros that exceed 80 columns. >>>> >>>> Examples: >>>> 1. >>>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/Mde= Pkg/Include/IndustryStandard/Acpi30.h#L702-#L704=20 >>>> >>>> >>>> 2. >>>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/Mde= Pkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031=20 >>>> >>>> >>>> >>>> There are other cases there as well. >>>> >>>> Anyway, if those were reduced in length, I think we could reach a=20 >>>> steady >>>> state. Some other minor tweaks might also be required. >>>> >>>> Regarding function calls, I put together the following branch to >>>> demonstrate some examples of what is done now. >>>> >>>> In summary, multiple arguments can be provided on a single line=20 >>>> (with no >>>> width or argument count maximum) or multiple lines. If a single=20 >>>> argument >>>> is multi-line, then all arguments must be on a unique line to=20 >>>> follow the >>>> multi-line argument convention. >>>> >>>> See the top two commits in this branch for examples: >>>> >>>> https://github.com/makubacki/edk2/tree/func_arg_formatting_demo >>>> >>>> I agree uncrustify and the spec be in sync. >>>> >>>> Regards, >>>> Michael >>>> >>>> On 11/14/2022 12:07 PM, Michael D Kinney wrote: >>>>> I disagree that they can coexist. >>>>> >>>>> If uncrustify is forcing 1 arg per line, then a developer that=20 >>>>> follows >>>>> a CSS that allows multiple per line, the code change will be rejected >>>>> by EDK II CI. >>>>> >>>>> The CSS and Uncristify behavior need to be aligned.If we want a CSS >>>>> change that requires Uncristify changes, then they have to be >>>>> coordinated. >>>>> >>>>> Mike >>>>> >>>>> *From:*devel@edk2.groups.io *On Behalf Of >>>>> *Chang, Abner via groups.io >>>>> *Sent:* Sunday, November 13, 2022 5:10 PM >>>>> *To:* devel@edk2.groups.io; Kinney, Michael D >>>>> ; Laszlo Ersek ; >>>>> Kubacki, Michael >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH >>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed >>>>> arguments >>>>> >>>>> [AMD Official Use Only - General] >>>>> >>>>> For this case, we don=E2=80=99t have to take another global reformatt= ing. >>>>> These two formats can coexisting without the conflict. =C2=A0We just = allow >>>>> the condense argus format in CSS. Also, update Uncrustify to not >>>>> forcing each argument at its own line. >>>>> >>>>> The current Uncrustify behavior seems to me match the CCS spec. But >>>>> this patch was sent to allow the multiple argus at the same line, >>>>> which was not proposed to fix the issue in current Uncrustify. You >>>>> sure we just close this issue? >>>>> >>>>> Abner >>>>> >>>>> *From:* devel@edk2.groups.io >>>>> > *On Behalf Of >>>>> *Michael D Kinney via groups.io >>>>> *Sent:* Monday, November 14, 2022 1:36 AM >>>>> *To:* devel@edk2.groups.io ; Chang,=20 >>>>> Abner >>>>> >; Laszlo Ersek >>>>> >; Kubacki, Michael >>>>> >>>> >; Kinney, Michael D >>>>> > >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH >>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed >>>>> arguments >>>>> >>>>> >>>>> >>>>> *Caution:*This message originated from an External Source. Use proper >>>>> caution when opening attachments, clicking links, or responding. >>>>> >>>>> We do not want another global format change because that make git >>>>> blame difficult to use. >>>>> >>>>> Are any clarifications required to describe the current Uncrustify >>>>> behavior?=C2=A0 Or is the description correct? >>>>> >>>>> If the current description matches Uncristify behavior, then I >>>>> recommend we close this issue as will not fix. >>>>> >>>>> Mike >>>>> >>>>> *From:* devel@edk2.groups.io >>>>> > *On Behalf Of >>>>> *Chang, Abner via groups.io >>>>> *Sent:* Sunday, November 13, 2022 12:45 AM >>>>> *To:* Kinney, Michael D >>>> >; devel@edk2.groups.io >>>>> ; Laszlo Ersek >>>> >; Kubacki, Michael >>>>> >>>> > >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH >>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed >>>>> arguments >>>>> >>>>> [AMD Official Use Only - General] >>>>> >>>>> Uncrustify can fix the first argument that is not at the indent with >>>>> two space. It also can fix the first argument that is not at the new >>>>> line. >>>>> >>>>> But it also makes each argument a new line if multiple args are >>>>> condensed in one line. That is what we have to update Uncrustify=20 >>>>> if we >>>>> have this patch merged to CCS. >>>>> >>>>> +Michael Kubacki in loop. >>>>> >>>>> Abner >>>>> >>>>> *From:* Kinney, Michael D >>>> > >>>>> *Sent:* Sunday, November 13, 2022 9:58 AM >>>>> *To:* devel@edk2.groups.io ; Chang,=20 >>>>> Abner >>>>> >; Laszlo Ersek >>>>> >; Kinney, Michael D >>>>> > >>>>> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH >>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed >>>>> arguments >>>>> >>>>> >>>>> >>>>> *Caution:*This message originated from an External Source. Use proper >>>>> caution when opening attachments, clicking links, or responding. >>>>> >>>>> Is this exactly what Uncrustify does now? >>>>> >>>>> Mike >>>>> >>>>> *From:* devel@edk2.groups.io >>>>> > *On Behalf Of >>>>> *Chang, Abner via groups.io >>>>> *Sent:* Saturday, November 12, 2022 5:36 PM >>>>> *To:* Laszlo Ersek >; >>>>> devel@edk2.groups.io >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH >>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed >>>>> arguments >>>>> >>>>> Hi all, >>>>> As we are going to release CCS 2.3, we would like to address some >>>>> pending issues of CCS. For this, I think we can, >>>>> - Still keep the one line per argument style in CCS although the >>>>> multi-arguments in the one line style can cover this. This avoids >>>>> confusion from readers and questions about if they can do the=20 >>>>> one-line >>>>> per argument style. >>>>> - If the arguments are in different lines, the first argument must be >>>>> indented with two spaces from the start of the function name or the >>>>> member function name. >>>>> How is this? >>>>> >>>>> Abner >>>>> >>>>> >>>> >>>> >>>> >>>> >>> >>> >>> >>> >> > > >=20 > >