From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web12.590.1668451757339678112 for ; Mon, 14 Nov 2022 10:49:17 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=YcPF+UEI; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 4BE3420B717A; Mon, 14 Nov 2022 10:49:16 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4BE3420B717A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1668451756; bh=dp8NJcN6hnnY3u89NupbsvBf108Kw0jQ3ikV20UdWfk=; h=Date:Subject:To:References:From:In-Reply-To:From; b=YcPF+UEIh+GkMSPGpBRPc+fDekP8WDA4tutdS7rNvGcGksYK1JQ1vgveabTlK0dRg 51vIF4eL6l9t+r/HIMYU4Ez0FLDWl9FHO0Bkw+WT31URTNsLMkA72/zgxkgN5CfzwD tyukWP6ZfrT6AYVySPbnPglxL/btlcbGBZsJNHcA= Message-ID: <7ed9dd39-71c2-f175-9ee9-f7f2f7b89e23@linux.microsoft.com> Date: Mon, 14 Nov 2022 13:49:15 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments To: "Kinney, Michael D" , "devel@edk2.groups.io" , "abner.chang@amd.com" , Laszlo Ersek , "Kubacki, Michael" References: <922996c2-e60a-80ec-6d4a-8b2e5a639c9b@redhat.com> <25893.1668303336748921539@groups.io> <17278424C4A5D78F.32003@groups.io> From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable 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 to=20 suggest that we raise more awareness about this change in a meeting with=20 members of the community to capture additional feedback. Also, the logistics (uncrustify aside) about how to execute this change=20 are unclear (as written in this thread and the BZ) and it would be=20 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, >=20 > Yes. They have been discussed. There were patches and discussion on mai= ling > lists back in 2017. Long before enabling uncrustify. I provided links to > these conversations in the following email about a week ago. >=20 > https://edk2.groups.io/g/devel/message/96038 >=20 >=20 > Mike >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Michael K= ubacki >> Sent: Monday, November 14, 2022 10:05 AM >> To: devel@edk2.groups.io; Kinney, Michael D = ; abner.chang@amd.com; Laszlo Ersek ; >> Kubacki, Michael >> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 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 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/Uncrustify= Check/uncrustify.cfg#L19. >>> >>> >>> 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 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 state= . >>> The issue after the second run is that uncrustify is confused about wha= t >>> to do with single macros that exceed 80 columns. >>> >>> Examples: >>> 1. >>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdeP= kg/Include/IndustryStandard/Acpi30.h#L702-#L704 >>> >>> 2. >>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdeP= kg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031 >>> >>> >>> There are other cases there as well. >>> >>> Anyway, if those were reduced in length, I think we could reach a stead= y >>> 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 (with n= o >>> width or argument count maximum) or multiple lines. If a single argumen= t >>> is multi-line, then all arguments must be on a unique line to follow th= e >>> 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 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 reformatti= ng. >>>> These two formats can coexisting without the conflict. =C2=A0We just a= llow >>>> 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, 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 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, 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 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 >> >=20