* Uncrustify configuration file and file/function templates
@ 2021-11-16 17:16 Michael D Kinney
2021-11-16 18:25 ` [edk2-devel] " Michael Kubacki
0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2021-11-16 17:16 UTC (permalink / raw)
To: devel@edk2.groups.io, Michael Kubacki
Hi Michael,
In your POC branch (https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
uncrustify.cfg configuration file in the root.
https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
However, in your Wiki, you provide examples where this configuration file is in an
.uncrustify directory
https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
The uncrustify.cfg files also contains commented out settings for the file header
and function header templates.
# cmt_insert_file_header = default_file_header.txt
# cmt_insert_func_header = default_function_header.txt
Are these disabled on purpose?
Do we want to enable them? If so, should the uncrustify configuration file
and the templates go into a .uncrustify directory?
Thanks,
Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 17:16 Uncrustify configuration file and file/function templates Michael D Kinney
@ 2021-11-16 18:25 ` Michael Kubacki
2021-11-16 18:31 ` Michael D Kinney
0 siblings, 1 reply; 12+ messages in thread
From: Michael Kubacki @ 2021-11-16 18:25 UTC (permalink / raw)
To: devel, michael.d.kinney
Hi Mike,
Those were just disabled because I typically run a separate invocation
of Uncrustify with them enabled to isolate code which is missing
file/function headers. My thought was the templates are helpful but we
would need to individually identify where they are placed to file TCBZs
for maintainers to replace the template with the actual information.
In some of my previous poc branches (like
https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers), I
also pushed a branch with those results.
So I do think we would want them enabled in the final config file. We
can also review the contents of the templates in the future patch series
to see if any changes are recommended.
I prefer using a .uncrustify directory to help group related collateral
but I don't have a strong opinion there.
Thanks,
Michael
On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> Hi Michael,
>
> In your POC branch (https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
> uncrustify.cfg configuration file in the root.
>
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
>
> However, in your Wiki, you provide examples where this configuration file is in an
> .uncrustify directory
>
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
>
> The uncrustify.cfg files also contains commented out settings for the file header
> and function header templates.
>
> # cmt_insert_file_header = default_file_header.txt
> # cmt_insert_func_header = default_function_header.txt
>
> Are these disabled on purpose?
>
> Do we want to enable them? If so, should the uncrustify configuration file
> and the templates go into a .uncrustify directory?
>
> Thanks,
>
> Mike
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 18:25 ` [edk2-devel] " Michael Kubacki
@ 2021-11-16 18:31 ` Michael D Kinney
2021-11-16 18:53 ` Michael Kubacki
0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2021-11-16 18:31 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com,
Kinney, Michael D
Hi Michael,
Should we have 2 versions of the config file?
One used by automation tools such as CI and git hooks that do not use the
templates.
And another one that a developer can optionally use that will add the
templates for missing file/function headers that the developer then needs
to fill out.
One concern I have about the templates is if they get used but a developer
does not fill in the missing information. It would be best if a CI check
rejects a file/function header that has not been filled in.
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Tuesday, November 16, 2021 10:25 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
>
> Hi Mike,
>
> Those were just disabled because I typically run a separate invocation
> of Uncrustify with them enabled to isolate code which is missing
> file/function headers. My thought was the templates are helpful but we
> would need to individually identify where they are placed to file TCBZs
> for maintainers to replace the template with the actual information.
>
> In some of my previous poc branches (like
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers), I
> also pushed a branch with those results.
>
> So I do think we would want them enabled in the final config file. We
> can also review the contents of the templates in the future patch series
> to see if any changes are recommended.
>
> I prefer using a .uncrustify directory to help group related collateral
> but I don't have a strong opinion there.
>
> Thanks,
> Michael
>
> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> > Hi Michael,
> >
> > In your POC branch (https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
> > uncrustify.cfg configuration file in the root.
> >
> > https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> >
> > However, in your Wiki, you provide examples where this configuration file is in an
> > .uncrustify directory
> >
> > https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >
> > The uncrustify.cfg files also contains commented out settings for the file header
> > and function header templates.
> >
> > # cmt_insert_file_header = default_file_header.txt
> > # cmt_insert_func_header = default_function_header.txt
> >
> > Are these disabled on purpose?
> >
> > Do we want to enable them? If so, should the uncrustify configuration file
> > and the templates go into a .uncrustify directory?
> >
> > Thanks,
> >
> > Mike
> >
> >
> >
> >
> >
> >
> >
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 18:31 ` Michael D Kinney
@ 2021-11-16 18:53 ` Michael Kubacki
2021-11-16 19:18 ` Michael D Kinney
0 siblings, 1 reply; 12+ messages in thread
From: Michael Kubacki @ 2021-11-16 18:53 UTC (permalink / raw)
To: devel, michael.d.kinney
I would prefer to have a single version of the file if possible to
reduce synchronization issues across the two copies.
It seems that a CI plugin to read the contents of the template files and
search incoming code for that text wouldn't be too difficult to add as a
new plugin.
Thanks,
Michael
On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> Hi Michael,
>
> Should we have 2 versions of the config file?
>
> One used by automation tools such as CI and git hooks that do not use the
> templates.
>
> And another one that a developer can optionally use that will add the
> templates for missing file/function headers that the developer then needs
> to fill out.
>
> One concern I have about the templates is if they get used but a developer
> does not fill in the missing information. It would be best if a CI check
> rejects a file/function header that has not been filled in.
>
> Mike
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
>> Sent: Tuesday, November 16, 2021 10:25 AM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
>> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
>>
>> Hi Mike,
>>
>> Those were just disabled because I typically run a separate invocation
>> of Uncrustify with them enabled to isolate code which is missing
>> file/function headers. My thought was the templates are helpful but we
>> would need to individually identify where they are placed to file TCBZs
>> for maintainers to replace the template with the actual information.
>>
>> In some of my previous poc branches (like
>> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers), I
>> also pushed a branch with those results.
>>
>> So I do think we would want them enabled in the final config file. We
>> can also review the contents of the templates in the future patch series
>> to see if any changes are recommended.
>>
>> I prefer using a .uncrustify directory to help group related collateral
>> but I don't have a strong opinion there.
>>
>> Thanks,
>> Michael
>>
>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
>>> Hi Michael,
>>>
>>> In your POC branch (https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
>>> uncrustify.cfg configuration file in the root.
>>>
>>> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
>>>
>>> However, in your Wiki, you provide examples where this configuration file is in an
>>> .uncrustify directory
>>>
>>> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
>>>
>>> The uncrustify.cfg files also contains commented out settings for the file header
>>> and function header templates.
>>>
>>> # cmt_insert_file_header = default_file_header.txt
>>> # cmt_insert_func_header = default_function_header.txt
>>>
>>> Are these disabled on purpose?
>>>
>>> Do we want to enable them? If so, should the uncrustify configuration file
>>> and the templates go into a .uncrustify directory?
>>>
>>> Thanks,
>>>
>>> Mike
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 18:53 ` Michael Kubacki
@ 2021-11-16 19:18 ` Michael D Kinney
2021-11-16 19:26 ` Michael Kubacki
0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2021-11-16 19:18 UTC (permalink / raw)
To: Michael Kubacki, devel@edk2.groups.io, Kinney, Michael D
Could we add this feature to the Uncrustify CI Plugin?
Mike
> -----Original Message-----
> From: Michael Kubacki <mikuback@linux.microsoft.com>
> Sent: Tuesday, November 16, 2021 10:54 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
>
> I would prefer to have a single version of the file if possible to
> reduce synchronization issues across the two copies.
>
> It seems that a CI plugin to read the contents of the template files and
> search incoming code for that text wouldn't be too difficult to add as a
> new plugin.
>
> Thanks,
> Michael
>
> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> > Hi Michael,
> >
> > Should we have 2 versions of the config file?
> >
> > One used by automation tools such as CI and git hooks that do not use the
> > templates.
> >
> > And another one that a developer can optionally use that will add the
> > templates for missing file/function headers that the developer then needs
> > to fill out.
> >
> > One concern I have about the templates is if they get used but a developer
> > does not fill in the missing information. It would be best if a CI check
> > rejects a file/function header that has not been filled in.
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> >> Sent: Tuesday, November 16, 2021 10:25 AM
> >> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> >> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
> >>
> >> Hi Mike,
> >>
> >> Those were just disabled because I typically run a separate invocation
> >> of Uncrustify with them enabled to isolate code which is missing
> >> file/function headers. My thought was the templates are helpful but we
> >> would need to individually identify where they are placed to file TCBZs
> >> for maintainers to replace the template with the actual information.
> >>
> >> In some of my previous poc branches (like
> >> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers), I
> >> also pushed a branch with those results.
> >>
> >> So I do think we would want them enabled in the final config file. We
> >> can also review the contents of the templates in the future patch series
> >> to see if any changes are recommended.
> >>
> >> I prefer using a .uncrustify directory to help group related collateral
> >> but I don't have a strong opinion there.
> >>
> >> Thanks,
> >> Michael
> >>
> >> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> >>> Hi Michael,
> >>>
> >>> In your POC branch (https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
> >>> uncrustify.cfg configuration file in the root.
> >>>
> >>> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> >>>
> >>> However, in your Wiki, you provide examples where this configuration file is in an
> >>> .uncrustify directory
> >>>
> >>> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >>>
> >>> The uncrustify.cfg files also contains commented out settings for the file header
> >>> and function header templates.
> >>>
> >>> # cmt_insert_file_header = default_file_header.txt
> >>> # cmt_insert_func_header = default_function_header.txt
> >>>
> >>> Are these disabled on purpose?
> >>>
> >>> Do we want to enable them? If so, should the uncrustify configuration file
> >>> and the templates go into a .uncrustify directory?
> >>>
> >>> Thanks,
> >>>
> >>> Mike
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 19:18 ` Michael D Kinney
@ 2021-11-16 19:26 ` Michael Kubacki
2021-11-16 23:05 ` Pedro Falcato
0 siblings, 1 reply; 12+ messages in thread
From: Michael Kubacki @ 2021-11-16 19:26 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
I think that makes sense. I will look into it further and let you know
if there's any downsides found.
On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> Could we add this feature to the Uncrustify CI Plugin?
>
> Mike
>
>> -----Original Message-----
>> From: Michael Kubacki <mikuback@linux.microsoft.com>
>> Sent: Tuesday, November 16, 2021 10:54 AM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
>> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
>>
>> I would prefer to have a single version of the file if possible to
>> reduce synchronization issues across the two copies.
>>
>> It seems that a CI plugin to read the contents of the template files and
>> search incoming code for that text wouldn't be too difficult to add as a
>> new plugin.
>>
>> Thanks,
>> Michael
>>
>> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
>>> Hi Michael,
>>>
>>> Should we have 2 versions of the config file?
>>>
>>> One used by automation tools such as CI and git hooks that do not use the
>>> templates.
>>>
>>> And another one that a developer can optionally use that will add the
>>> templates for missing file/function headers that the developer then needs
>>> to fill out.
>>>
>>> One concern I have about the templates is if they get used but a developer
>>> does not fill in the missing information. It would be best if a CI check
>>> rejects a file/function header that has not been filled in.
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
>>>> Sent: Tuesday, November 16, 2021 10:25 AM
>>>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
>>>> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
>>>>
>>>> Hi Mike,
>>>>
>>>> Those were just disabled because I typically run a separate invocation
>>>> of Uncrustify with them enabled to isolate code which is missing
>>>> file/function headers. My thought was the templates are helpful but we
>>>> would need to individually identify where they are placed to file TCBZs
>>>> for maintainers to replace the template with the actual information.
>>>>
>>>> In some of my previous poc branches (like
>>>> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers), I
>>>> also pushed a branch with those results.
>>>>
>>>> So I do think we would want them enabled in the final config file. We
>>>> can also review the contents of the templates in the future patch series
>>>> to see if any changes are recommended.
>>>>
>>>> I prefer using a .uncrustify directory to help group related collateral
>>>> but I don't have a strong opinion there.
>>>>
>>>> Thanks,
>>>> Michael
>>>>
>>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
>>>>> Hi Michael,
>>>>>
>>>>> In your POC branch (https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
>>>>> uncrustify.cfg configuration file in the root.
>>>>>
>>>>> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
>>>>>
>>>>> However, in your Wiki, you provide examples where this configuration file is in an
>>>>> .uncrustify directory
>>>>>
>>>>> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
>>>>>
>>>>> The uncrustify.cfg files also contains commented out settings for the file header
>>>>> and function header templates.
>>>>>
>>>>> # cmt_insert_file_header = default_file_header.txt
>>>>> # cmt_insert_func_header = default_function_header.txt
>>>>>
>>>>> Are these disabled on purpose?
>>>>>
>>>>> Do we want to enable them? If so, should the uncrustify configuration file
>>>>> and the templates go into a .uncrustify directory?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Mike
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 19:26 ` Michael Kubacki
@ 2021-11-16 23:05 ` Pedro Falcato
2021-11-16 23:20 ` Michael Kubacki
0 siblings, 1 reply; 12+ messages in thread
From: Pedro Falcato @ 2021-11-16 23:05 UTC (permalink / raw)
To: edk2-devel-groups-io, mikuback
[-- Attachment #1: Type: text/plain, Size: 4960 bytes --]
Hi,
RE: Uncrustify's configuration
I had made a few changes to the config file, when trying out Uncrustify, a
few months ago, on Mike Kinney's suggestion. In my experience, the new
config file reflects the edk2 coding style better.
In particular, it adds spaces between things like 'if' and the parenthesis,
and better aligns functions' parameters.
Feel free to check out the diff:
https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2. There's a
bit of diff noise due to it being based on
https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg but
diffed on
https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
.
Thanks,
Pedro
On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki <
mikuback@linux.microsoft.com> wrote:
> I think that makes sense. I will look into it further and let you know
> if there's any downsides found.
>
> On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> > Could we add this feature to the Uncrustify CI Plugin?
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Michael Kubacki <mikuback@linux.microsoft.com>
> >> Sent: Tuesday, November 16, 2021 10:54 AM
> >> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com
> >
> >> Subject: Re: [edk2-devel] Uncrustify configuration file and
> file/function templates
> >>
> >> I would prefer to have a single version of the file if possible to
> >> reduce synchronization issues across the two copies.
> >>
> >> It seems that a CI plugin to read the contents of the template files and
> >> search incoming code for that text wouldn't be too difficult to add as a
> >> new plugin.
> >>
> >> Thanks,
> >> Michael
> >>
> >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> >>> Hi Michael,
> >>>
> >>> Should we have 2 versions of the config file?
> >>>
> >>> One used by automation tools such as CI and git hooks that do not use
> the
> >>> templates.
> >>>
> >>> And another one that a developer can optionally use that will add the
> >>> templates for missing file/function headers that the developer then
> needs
> >>> to fill out.
> >>>
> >>> One concern I have about the templates is if they get used but a
> developer
> >>> does not fill in the missing information. It would be best if a CI
> check
> >>> rejects a file/function header that has not been filled in.
> >>>
> >>> Mike
> >>>
> >>>> -----Original Message-----
> >>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Michael Kubacki
> >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> >>>> To: devel@edk2.groups.io; Kinney, Michael D <
> michael.d.kinney@intel.com>
> >>>> Subject: Re: [edk2-devel] Uncrustify configuration file and
> file/function templates
> >>>>
> >>>> Hi Mike,
> >>>>
> >>>> Those were just disabled because I typically run a separate invocation
> >>>> of Uncrustify with them enabled to isolate code which is missing
> >>>> file/function headers. My thought was the templates are helpful but we
> >>>> would need to individually identify where they are placed to file
> TCBZs
> >>>> for maintainers to replace the template with the actual information.
> >>>>
> >>>> In some of my previous poc branches (like
> >>>>
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers),
> I
> >>>> also pushed a branch with those results.
> >>>>
> >>>> So I do think we would want them enabled in the final config file. We
> >>>> can also review the contents of the templates in the future patch
> series
> >>>> to see if any changes are recommended.
> >>>>
> >>>> I prefer using a .uncrustify directory to help group related
> collateral
> >>>> but I don't have a strong opinion there.
> >>>>
> >>>> Thanks,
> >>>> Michael
> >>>>
> >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> >>>>> Hi Michael,
> >>>>>
> >>>>> In your POC branch (
> https://github.com/makubacki/edk2/tree/uncrustify_poc_5), I see the
> >>>>> uncrustify.cfg configuration file in the root.
> >>>>>
> >>>>>
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> >>>>>
> >>>>> However, in your Wiki, you provide examples where this configuration
> file is in an
> >>>>> .uncrustify directory
> >>>>>
> >>>>>
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >>>>>
> >>>>> The uncrustify.cfg files also contains commented out settings for
> the file header
> >>>>> and function header templates.
> >>>>>
> >>>>> # cmt_insert_file_header = default_file_header.txt
> >>>>> # cmt_insert_func_header =
> default_function_header.txt
> >>>>>
> >>>>> Are these disabled on purpose?
> >>>>>
> >>>>> Do we want to enable them? If so, should the uncrustify
> configuration file
> >>>>> and the templates go into a .uncrustify directory?
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
>
>
>
>
>
>
--
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 8402 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 23:05 ` Pedro Falcato
@ 2021-11-16 23:20 ` Michael Kubacki
2021-11-16 23:30 ` Pedro Falcato
0 siblings, 1 reply; 12+ messages in thread
From: Michael Kubacki @ 2021-11-16 23:20 UTC (permalink / raw)
To: devel, pedro.falcato
Hi Pedro,
By "new config", do you mean the config file in the latest branch
https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg)?
The main difference I see from the diff in that gist and poc_5 is that
"align_func_params_gap" and "align_func_params_span" are two instead of
zero.
The value "force" instead of "add" for "sp_before_sparen" will ensure a
single space is always added between a control statement and the
parenthesis.
Thanks,
Michael
On 11/16/2021 6:05 PM, Pedro Falcato wrote:
> Hi,
>
> RE: Uncrustify's configuration
>
> I had made a few changes to the config file, when trying out Uncrustify,
> a few months ago, on Mike Kinney's suggestion. In my experience, the new
> config file reflects the edk2 coding style better.
>
> In particular, it adds spaces between things like 'if' and the
> parenthesis, and better aligns functions' parameters.
> Feel free to check out the diff:
> https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>.
> There's a bit of diff noise due to it being based on
> https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg> but
> diffed on
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg>.
>
> Thanks,
> Pedro
>
> On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki
> <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>> wrote:
>
> I think that makes sense. I will look into it further and let you know
> if there's any downsides found.
>
> On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> > Could we add this feature to the Uncrustify CI Plugin?
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Michael Kubacki <mikuback@linux.microsoft.com
> <mailto:mikuback@linux.microsoft.com>>
> >> Sent: Tuesday, November 16, 2021 10:54 AM
> >> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Kinney,
> Michael D <michael.d.kinney@intel.com
> <mailto:michael.d.kinney@intel.com>>
> >> Subject: Re: [edk2-devel] Uncrustify configuration file and
> file/function templates
> >>
> >> I would prefer to have a single version of the file if possible to
> >> reduce synchronization issues across the two copies.
> >>
> >> It seems that a CI plugin to read the contents of the template
> files and
> >> search incoming code for that text wouldn't be too difficult to
> add as a
> >> new plugin.
> >>
> >> Thanks,
> >> Michael
> >>
> >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> >>> Hi Michael,
> >>>
> >>> Should we have 2 versions of the config file?
> >>>
> >>> One used by automation tools such as CI and git hooks that do
> not use the
> >>> templates.
> >>>
> >>> And another one that a developer can optionally use that will
> add the
> >>> templates for missing file/function headers that the developer
> then needs
> >>> to fill out.
> >>>
> >>> One concern I have about the templates is if they get used but
> a developer
> >>> does not fill in the missing information. It would be best if
> a CI check
> >>> rejects a file/function header that has not been filled in.
> >>>
> >>> Mike
> >>>
> >>>> -----Original Message-----
> >>>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of
> Michael Kubacki
> >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> >>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>;
> Kinney, Michael D <michael.d.kinney@intel.com
> <mailto:michael.d.kinney@intel.com>>
> >>>> Subject: Re: [edk2-devel] Uncrustify configuration file and
> file/function templates
> >>>>
> >>>> Hi Mike,
> >>>>
> >>>> Those were just disabled because I typically run a separate
> invocation
> >>>> of Uncrustify with them enabled to isolate code which is missing
> >>>> file/function headers. My thought was the templates are
> helpful but we
> >>>> would need to individually identify where they are placed to
> file TCBZs
> >>>> for maintainers to replace the template with the actual
> information.
> >>>>
> >>>> In some of my previous poc branches (like
> >>>>
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>),
> I
> >>>> also pushed a branch with those results.
> >>>>
> >>>> So I do think we would want them enabled in the final config
> file. We
> >>>> can also review the contents of the templates in the future
> patch series
> >>>> to see if any changes are recommended.
> >>>>
> >>>> I prefer using a .uncrustify directory to help group related
> collateral
> >>>> but I don't have a strong opinion there.
> >>>>
> >>>> Thanks,
> >>>> Michael
> >>>>
> >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> >>>>> Hi Michael,
> >>>>>
> >>>>> In your POC branch
> (https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>), I see the
> >>>>> uncrustify.cfg configuration file in the root.
> >>>>>
> >>>>>
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>
> >>>>>
> >>>>> However, in your Wiki, you provide examples where this
> configuration file is in an
> >>>>> .uncrustify directory
> >>>>>
> >>>>>
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme>
> >>>>>
> >>>>> The uncrustify.cfg files also contains commented out settings
> for the file header
> >>>>> and function header templates.
> >>>>>
> >>>>> # cmt_insert_file_header =
> default_file_header.txt
> >>>>> # cmt_insert_func_header =
> default_function_header.txt
> >>>>>
> >>>>> Are these disabled on purpose?
> >>>>>
> >>>>> Do we want to enable them? If so, should the uncrustify
> configuration file
> >>>>> and the templates go into a .uncrustify directory?
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
>
>
>
>
>
>
>
> --
> Pedro Falcato
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 23:20 ` Michael Kubacki
@ 2021-11-16 23:30 ` Pedro Falcato
2021-11-16 23:54 ` Michael Kubacki
0 siblings, 1 reply; 12+ messages in thread
From: Pedro Falcato @ 2021-11-16 23:30 UTC (permalink / raw)
To: Michael Kubacki; +Cc: edk2-devel-groups-io
[-- Attachment #1: Type: text/plain, Size: 7947 bytes --]
Yes, I do.
Essentially, my changes make it so the function parameters are aligned and
there are always two spaces between the parameter's type and name; it also
adds spaces between control statements, as you mentioned.
"force" instead of "add" for sp_before_sparen is a good idea though.
I assume these are good additions to the "upstream" edk2.cfg?
On Tue, Nov 16, 2021 at 11:20 PM Michael Kubacki <
mikuback@linux.microsoft.com> wrote:
> Hi Pedro,
>
> By "new config", do you mean the config file in the latest branch
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg)?
>
> The main difference I see from the diff in that gist and poc_5 is that
> "align_func_params_gap" and "align_func_params_span" are two instead of
> zero.
>
> The value "force" instead of "add" for "sp_before_sparen" will ensure a
> single space is always added between a control statement and the
> parenthesis.
>
> Thanks,
> Michael
>
> On 11/16/2021 6:05 PM, Pedro Falcato wrote:
> > Hi,
> >
> > RE: Uncrustify's configuration
> >
> > I had made a few changes to the config file, when trying out Uncrustify,
> > a few months ago, on Mike Kinney's suggestion. In my experience, the new
> > config file reflects the edk2 coding style better.
> >
> > In particular, it adds spaces between things like 'if' and the
> > parenthesis, and better aligns functions' parameters.
> > Feel free to check out the diff:
> > https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>.
> > There's a bit of diff noise due to it being based on
> > https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>
> but
> > diffed on
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> >.
> >
> > Thanks,
> > Pedro
> >
> > On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki
> > <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>>
> wrote:
> >
> > I think that makes sense. I will look into it further and let you
> know
> > if there's any downsides found.
> >
> > On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> > > Could we add this feature to the Uncrustify CI Plugin?
> > >
> > > Mike
> > >
> > >> -----Original Message-----
> > >> From: Michael Kubacki <mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>>
> > >> Sent: Tuesday, November 16, 2021 10:54 AM
> > >> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Kinney,
> > Michael D <michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>>
> > >> Subject: Re: [edk2-devel] Uncrustify configuration file and
> > file/function templates
> > >>
> > >> I would prefer to have a single version of the file if possible
> to
> > >> reduce synchronization issues across the two copies.
> > >>
> > >> It seems that a CI plugin to read the contents of the template
> > files and
> > >> search incoming code for that text wouldn't be too difficult to
> > add as a
> > >> new plugin.
> > >>
> > >> Thanks,
> > >> Michael
> > >>
> > >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> > >>> Hi Michael,
> > >>>
> > >>> Should we have 2 versions of the config file?
> > >>>
> > >>> One used by automation tools such as CI and git hooks that do
> > not use the
> > >>> templates.
> > >>>
> > >>> And another one that a developer can optionally use that will
> > add the
> > >>> templates for missing file/function headers that the developer
> > then needs
> > >>> to fill out.
> > >>>
> > >>> One concern I have about the templates is if they get used but
> > a developer
> > >>> does not fill in the missing information. It would be best if
> > a CI check
> > >>> rejects a file/function header that has not been filled in.
> > >>>
> > >>> Mike
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of
> > Michael Kubacki
> > >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> > >>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>;
> > Kinney, Michael D <michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>>
> > >>>> Subject: Re: [edk2-devel] Uncrustify configuration file and
> > file/function templates
> > >>>>
> > >>>> Hi Mike,
> > >>>>
> > >>>> Those were just disabled because I typically run a separate
> > invocation
> > >>>> of Uncrustify with them enabled to isolate code which is
> missing
> > >>>> file/function headers. My thought was the templates are
> > helpful but we
> > >>>> would need to individually identify where they are placed to
> > file TCBZs
> > >>>> for maintainers to replace the template with the actual
> > information.
> > >>>>
> > >>>> In some of my previous poc branches (like
> > >>>>
> >
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> > <
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>),
> > I
> > >>>> also pushed a branch with those results.
> > >>>>
> > >>>> So I do think we would want them enabled in the final config
> > file. We
> > >>>> can also review the contents of the templates in the future
> > patch series
> > >>>> to see if any changes are recommended.
> > >>>>
> > >>>> I prefer using a .uncrustify directory to help group related
> > collateral
> > >>>> but I don't have a strong opinion there.
> > >>>>
> > >>>> Thanks,
> > >>>> Michael
> > >>>>
> > >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> > >>>>> Hi Michael,
> > >>>>>
> > >>>>> In your POC branch
> > (https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>), I see
> the
> > >>>>> uncrustify.cfg configuration file in the root.
> > >>>>>
> > >>>>>
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>
> > >>>>>
> > >>>>> However, in your Wiki, you provide examples where this
> > configuration file is in an
> > >>>>> .uncrustify directory
> > >>>>>
> > >>>>>
> >
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> > <
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >
> > >>>>>
> > >>>>> The uncrustify.cfg files also contains commented out settings
> > for the file header
> > >>>>> and function header templates.
> > >>>>>
> > >>>>> # cmt_insert_file_header =
> > default_file_header.txt
> > >>>>> # cmt_insert_func_header =
> > default_function_header.txt
> > >>>>>
> > >>>>> Are these disabled on purpose?
> > >>>>>
> > >>>>> Do we want to enable them? If so, should the uncrustify
> > configuration file
> > >>>>> and the templates go into a .uncrustify directory?
> > >>>>>
> > >>>>> Thanks,
> > >>>>>
> > >>>>> Mike
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> >
> >
> >
> >
> >
> >
> >
> > --
> > Pedro Falcato
> >
>
--
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 14260 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 23:30 ` Pedro Falcato
@ 2021-11-16 23:54 ` Michael Kubacki
2021-11-17 0:32 ` Michael D Kinney
2021-11-17 1:46 ` Pedro Falcato
0 siblings, 2 replies; 12+ messages in thread
From: Michael Kubacki @ 2021-11-16 23:54 UTC (permalink / raw)
To: Pedro Falcato; +Cc: edk2-devel-groups-io
The poc_5 branch config file is based on the file I'm pushing to the
Uncrustify fork soon:
https://dev.azure.com/projectmu/Uncrustify/_git/Uncrustify/pullrequest/24?_a=files&path=/etc/edk2.cfg
So they're already being used and the file in the poc_5 branch was of
course used to generate the results in the branch.
Note that I do have "align_func_params" set to true. I believe I saw
some occasional anomalies in the past when I set
"align_func_params_span" and the gap to larger values but I don't recall
exactly what they were. I'd be happy to try that again soon though.
On 11/16/2021 6:30 PM, Pedro Falcato wrote:
> Yes, I do.
>
> Essentially, my changes make it so the function parameters are aligned
> and there are always two spaces between the parameter's type and name;
> it also adds spaces between control statements, as you mentioned.
> "force" instead of "add" for sp_before_sparen is a good idea though.
> I assume these are good additions to the "upstream" edk2.cfg?
>
> On Tue, Nov 16, 2021 at 11:20 PM Michael Kubacki
> <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>> wrote:
>
> Hi Pedro,
>
> By "new config", do you mean the config file in the latest branch
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>)?
>
> The main difference I see from the diff in that gist and poc_5 is that
> "align_func_params_gap" and "align_func_params_span" are two instead of
> zero.
>
> The value "force" instead of "add" for "sp_before_sparen" will ensure a
> single space is always added between a control statement and the
> parenthesis.
>
> Thanks,
> Michael
>
> On 11/16/2021 6:05 PM, Pedro Falcato wrote:
> > Hi,
> >
> > RE: Uncrustify's configuration
> >
> > I had made a few changes to the config file, when trying out
> Uncrustify,
> > a few months ago, on Mike Kinney's suggestion. In my experience,
> the new
> > config file reflects the edk2 coding style better.
> >
> > In particular, it adds spaces between things like 'if' and the
> > parenthesis, and better aligns functions' parameters.
> > Feel free to check out the diff:
> > https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>>.
> > There's a bit of diff noise due to it being based on
> >
> https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>
> >
> <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>> but
>
> > diffed on
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg>
>
> >
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg>>.
> >
> > Thanks,
> > Pedro
> >
> > On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki
> > <mikuback@linux.microsoft.com
> <mailto:mikuback@linux.microsoft.com>
> <mailto:mikuback@linux.microsoft.com
> <mailto:mikuback@linux.microsoft.com>>> wrote:
> >
> > I think that makes sense. I will look into it further and let
> you know
> > if there's any downsides found.
> >
> > On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> > > Could we add this feature to the Uncrustify CI Plugin?
> > >
> > > Mike
> > >
> > >> -----Original Message-----
> > >> From: Michael Kubacki <mikuback@linux.microsoft.com
> <mailto:mikuback@linux.microsoft.com>
> > <mailto:mikuback@linux.microsoft.com
> <mailto:mikuback@linux.microsoft.com>>>
> > >> Sent: Tuesday, November 16, 2021 10:54 AM
> > >> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>; Kinney,
> > Michael D <michael.d.kinney@intel.com
> <mailto:michael.d.kinney@intel.com>
> > <mailto:michael.d.kinney@intel.com
> <mailto:michael.d.kinney@intel.com>>>
> > >> Subject: Re: [edk2-devel] Uncrustify configuration file and
> > file/function templates
> > >>
> > >> I would prefer to have a single version of the file if
> possible to
> > >> reduce synchronization issues across the two copies.
> > >>
> > >> It seems that a CI plugin to read the contents of the
> template
> > files and
> > >> search incoming code for that text wouldn't be too
> difficult to
> > add as a
> > >> new plugin.
> > >>
> > >> Thanks,
> > >> Michael
> > >>
> > >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> > >>> Hi Michael,
> > >>>
> > >>> Should we have 2 versions of the config file?
> > >>>
> > >>> One used by automation tools such as CI and git hooks
> that do
> > not use the
> > >>> templates.
> > >>>
> > >>> And another one that a developer can optionally use that
> will
> > add the
> > >>> templates for missing file/function headers that the
> developer
> > then needs
> > >>> to fill out.
> > >>>
> > >>> One concern I have about the templates is if they get
> used but
> > a developer
> > >>> does not fill in the missing information. It would be
> best if
> > a CI check
> > >>> rejects a file/function header that has not been filled in.
> > >>>
> > >>> Mike
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: devel@edk2.groups.io
> <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io
> <mailto:devel@edk2.groups.io>>
> > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>> On
> Behalf Of
> > Michael Kubacki
> > >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> > >>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>;
> > Kinney, Michael D <michael.d.kinney@intel.com
> <mailto:michael.d.kinney@intel.com>
> > <mailto:michael.d.kinney@intel.com
> <mailto:michael.d.kinney@intel.com>>>
> > >>>> Subject: Re: [edk2-devel] Uncrustify configuration file and
> > file/function templates
> > >>>>
> > >>>> Hi Mike,
> > >>>>
> > >>>> Those were just disabled because I typically run a separate
> > invocation
> > >>>> of Uncrustify with them enabled to isolate code which
> is missing
> > >>>> file/function headers. My thought was the templates are
> > helpful but we
> > >>>> would need to individually identify where they are
> placed to
> > file TCBZs
> > >>>> for maintainers to replace the template with the actual
> > information.
> > >>>>
> > >>>> In some of my previous poc branches (like
> > >>>>
> >
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>
> >
> <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>>),
> > I
> > >>>> also pushed a branch with those results.
> > >>>>
> > >>>> So I do think we would want them enabled in the final
> config
> > file. We
> > >>>> can also review the contents of the templates in the future
> > patch series
> > >>>> to see if any changes are recommended.
> > >>>>
> > >>>> I prefer using a .uncrustify directory to help group
> related
> > collateral
> > >>>> but I don't have a strong opinion there.
> > >>>>
> > >>>> Thanks,
> > >>>> Michael
> > >>>>
> > >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> > >>>>> Hi Michael,
> > >>>>>
> > >>>>> In your POC branch
> > (https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>
> > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>>), I see the
> > >>>>> uncrustify.cfg configuration file in the root.
> > >>>>>
> > >>>>>
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>
> >
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>>
> > >>>>>
> > >>>>> However, in your Wiki, you provide examples where this
> > configuration file is in an
> > >>>>> .uncrustify directory
> > >>>>>
> > >>>>>
> >
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme>
> >
> <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme>>
> > >>>>>
> > >>>>> The uncrustify.cfg files also contains commented out
> settings
> > for the file header
> > >>>>> and function header templates.
> > >>>>>
> > >>>>> # cmt_insert_file_header =
> > default_file_header.txt
> > >>>>> # cmt_insert_func_header =
> > default_function_header.txt
> > >>>>>
> > >>>>> Are these disabled on purpose?
> > >>>>>
> > >>>>> Do we want to enable them? If so, should the uncrustify
> > configuration file
> > >>>>> and the templates go into a .uncrustify directory?
> > >>>>>
> > >>>>> Thanks,
> > >>>>>
> > >>>>> Mike
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> >
> >
> >
> >
> >
> >
> >
> > --
> > Pedro Falcato
> >
>
>
>
> --
> Pedro Falcato
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 23:54 ` Michael Kubacki
@ 2021-11-17 0:32 ` Michael D Kinney
2021-11-17 1:46 ` Pedro Falcato
1 sibling, 0 replies; 12+ messages in thread
From: Michael D Kinney @ 2021-11-17 0:32 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com, Pedro Falcato,
Kinney, Michael D
Hi Michael,
I have created a PR based on your poc_5 branch work using the latest edk2/master
and have started an EDK II CI pass.
https://github.com/tianocore/edk2/pull/2208
This PR can also be used to easily review the changes that uncrustify has made to
the source style of all edk2 packages.
I have also created a BZ for applying the uncrustify changes.
https://bugzilla.tianocore.org/show_bug.cgi?id=3737
I am already seen some PatchCheck and ECC issues with this PR. I will review
and provide recommendations in the BZ.
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Tuesday, November 16, 2021 3:55 PM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>
> Subject: Re: [edk2-devel] Uncrustify configuration file and file/function templates
>
> The poc_5 branch config file is based on the file I'm pushing to the
> Uncrustify fork soon:
> https://dev.azure.com/projectmu/Uncrustify/_git/Uncrustify/pullrequest/24?_a=files&path=/etc/edk2.cfg
>
> So they're already being used and the file in the poc_5 branch was of
> course used to generate the results in the branch.
>
> Note that I do have "align_func_params" set to true. I believe I saw
> some occasional anomalies in the past when I set
> "align_func_params_span" and the gap to larger values but I don't recall
> exactly what they were. I'd be happy to try that again soon though.
>
> On 11/16/2021 6:30 PM, Pedro Falcato wrote:
> > Yes, I do.
> >
> > Essentially, my changes make it so the function parameters are aligned
> > and there are always two spaces between the parameter's type and name;
> > it also adds spaces between control statements, as you mentioned.
> > "force" instead of "add" for sp_before_sparen is a good idea though.
> > I assume these are good additions to the "upstream" edk2.cfg?
> >
> > On Tue, Nov 16, 2021 at 11:20 PM Michael Kubacki
> > <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>> wrote:
> >
> > Hi Pedro,
> >
> > By "new config", do you mean the config file in the latest branch
> > https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> > <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>)?
> >
> > The main difference I see from the diff in that gist and poc_5 is that
> > "align_func_params_gap" and "align_func_params_span" are two instead of
> > zero.
> >
> > The value "force" instead of "add" for "sp_before_sparen" will ensure a
> > single space is always added between a control statement and the
> > parenthesis.
> >
> > Thanks,
> > Michael
> >
> > On 11/16/2021 6:05 PM, Pedro Falcato wrote:
> > > Hi,
> > >
> > > RE: Uncrustify's configuration
> > >
> > > I had made a few changes to the config file, when trying out
> > Uncrustify,
> > > a few months ago, on Mike Kinney's suggestion. In my experience,
> > the new
> > > config file reflects the edk2 coding style better.
> > >
> > > In particular, it adds spaces between things like 'if' and the
> > > parenthesis, and better aligns functions' parameters.
> > > Feel free to check out the diff:
> > > https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>
> > > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>>.
> > > There's a bit of diff noise due to it being based on
> > >
> > https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>
> > >
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>> but
> >
> > > diffed on
> > >
> > https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> > <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg>
> >
> > >
> > <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> > <https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg>>.
> > >
> > > Thanks,
> > > Pedro
> > >
> > > On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki
> > > <mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>
> > <mailto:mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>>> wrote:
> > >
> > > I think that makes sense. I will look into it further and let
> > you know
> > > if there's any downsides found.
> > >
> > > On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> > > > Could we add this feature to the Uncrustify CI Plugin?
> > > >
> > > > Mike
> > > >
> > > >> -----Original Message-----
> > > >> From: Michael Kubacki <mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>
> > > <mailto:mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>>>
> > > >> Sent: Tuesday, November 16, 2021 10:54 AM
> > > >> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>; Kinney,
> > > Michael D <michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>
> > > <mailto:michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>>>
> > > >> Subject: Re: [edk2-devel] Uncrustify configuration file and
> > > file/function templates
> > > >>
> > > >> I would prefer to have a single version of the file if
> > possible to
> > > >> reduce synchronization issues across the two copies.
> > > >>
> > > >> It seems that a CI plugin to read the contents of the
> > template
> > > files and
> > > >> search incoming code for that text wouldn't be too
> > difficult to
> > > add as a
> > > >> new plugin.
> > > >>
> > > >> Thanks,
> > > >> Michael
> > > >>
> > > >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> > > >>> Hi Michael,
> > > >>>
> > > >>> Should we have 2 versions of the config file?
> > > >>>
> > > >>> One used by automation tools such as CI and git hooks
> > that do
> > > not use the
> > > >>> templates.
> > > >>>
> > > >>> And another one that a developer can optionally use that
> > will
> > > add the
> > > >>> templates for missing file/function headers that the
> > developer
> > > then needs
> > > >>> to fill out.
> > > >>>
> > > >>> One concern I have about the templates is if they get
> > used but
> > > a developer
> > > >>> does not fill in the missing information. It would be
> > best if
> > > a CI check
> > > >>> rejects a file/function header that has not been filled in.
> > > >>>
> > > >>> Mike
> > > >>>
> > > >>>> -----Original Message-----
> > > >>>> From: devel@edk2.groups.io
> > <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io
> > <mailto:devel@edk2.groups.io>>
> > > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>> On
> > Behalf Of
> > > Michael Kubacki
> > > >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> > > >>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>;
> > > Kinney, Michael D <michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>
> > > <mailto:michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>>>
> > > >>>> Subject: Re: [edk2-devel] Uncrustify configuration file and
> > > file/function templates
> > > >>>>
> > > >>>> Hi Mike,
> > > >>>>
> > > >>>> Those were just disabled because I typically run a separate
> > > invocation
> > > >>>> of Uncrustify with them enabled to isolate code which
> > is missing
> > > >>>> file/function headers. My thought was the templates are
> > > helpful but we
> > > >>>> would need to individually identify where they are
> > placed to
> > > file TCBZs
> > > >>>> for maintainers to replace the template with the actual
> > > information.
> > > >>>>
> > > >>>> In some of my previous poc branches (like
> > > >>>>
> > >
> > https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> > <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>
> > >
> > <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> <https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>>),
> > > I
> > > >>>> also pushed a branch with those results.
> > > >>>>
> > > >>>> So I do think we would want them enabled in the final
> > config
> > > file. We
> > > >>>> can also review the contents of the templates in the future
> > > patch series
> > > >>>> to see if any changes are recommended.
> > > >>>>
> > > >>>> I prefer using a .uncrustify directory to help group
> > related
> > > collateral
> > > >>>> but I don't have a strong opinion there.
> > > >>>>
> > > >>>> Thanks,
> > > >>>> Michael
> > > >>>>
> > > >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> > > >>>>> Hi Michael,
> > > >>>>>
> > > >>>>> In your POC branch
> > > (https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>
> > > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>>), I see the
> > > >>>>> uncrustify.cfg configuration file in the root.
> > > >>>>>
> > > >>>>>
> > >
> > https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> > <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>
> > >
> > <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> <https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>>
> > > >>>>>
> > > >>>>> However, in your Wiki, you provide examples where this
> > > configuration file is in an
> > > >>>>> .uncrustify directory
> > > >>>>>
> > > >>>>>
> > >
> > https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> > <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme>
> > >
> > <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme>>
> > > >>>>>
> > > >>>>> The uncrustify.cfg files also contains commented out
> > settings
> > > for the file header
> > > >>>>> and function header templates.
> > > >>>>>
> > > >>>>> # cmt_insert_file_header =
> > > default_file_header.txt
> > > >>>>> # cmt_insert_func_header =
> > > default_function_header.txt
> > > >>>>>
> > > >>>>> Are these disabled on purpose?
> > > >>>>>
> > > >>>>> Do we want to enable them? If so, should the uncrustify
> > > configuration file
> > > >>>>> and the templates go into a .uncrustify directory?
> > > >>>>>
> > > >>>>> Thanks,
> > > >>>>>
> > > >>>>> Mike
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > --
> > > Pedro Falcato
> > >
> >
> >
> >
> > --
> > Pedro Falcato
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] Uncrustify configuration file and file/function templates
2021-11-16 23:54 ` Michael Kubacki
2021-11-17 0:32 ` Michael D Kinney
@ 2021-11-17 1:46 ` Pedro Falcato
1 sibling, 0 replies; 12+ messages in thread
From: Pedro Falcato @ 2021-11-17 1:46 UTC (permalink / raw)
To: Michael Kubacki; +Cc: edk2-devel-groups-io
[-- Attachment #1: Type: text/plain, Size: 12592 bytes --]
If you're curious, you can look at my Ext4Pkg (
https://github.com/tianocore/edk2-platforms/tree/master/Features/Ext4Pkg/Ext4Dxe)
to look at the effects of align_func_params_gap and align_func_params_span
= 2. Things look
okay and similar to other edk2-styled code.
Best regards,
Pedro
On Tue, Nov 16, 2021 at 11:54 PM Michael Kubacki <
mikuback@linux.microsoft.com> wrote:
> The poc_5 branch config file is based on the file I'm pushing to the
> Uncrustify fork soon:
>
> https://dev.azure.com/projectmu/Uncrustify/_git/Uncrustify/pullrequest/24?_a=files&path=/etc/edk2.cfg
>
> So they're already being used and the file in the poc_5 branch was of
> course used to generate the results in the branch.
>
> Note that I do have "align_func_params" set to true. I believe I saw
> some occasional anomalies in the past when I set
> "align_func_params_span" and the gap to larger values but I don't recall
> exactly what they were. I'd be happy to try that again soon though.
>
> On 11/16/2021 6:30 PM, Pedro Falcato wrote:
> > Yes, I do.
> >
> > Essentially, my changes make it so the function parameters are aligned
> > and there are always two spaces between the parameter's type and name;
> > it also adds spaces between control statements, as you mentioned.
> > "force" instead of "add" for sp_before_sparen is a good idea though.
> > I assume these are good additions to the "upstream" edk2.cfg?
> >
> > On Tue, Nov 16, 2021 at 11:20 PM Michael Kubacki
> > <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>>
> wrote:
> >
> > Hi Pedro,
> >
> > By "new config", do you mean the config file in the latest branch
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>)?
> >
> > The main difference I see from the diff in that gist and poc_5 is
> that
> > "align_func_params_gap" and "align_func_params_span" are two instead
> of
> > zero.
> >
> > The value "force" instead of "add" for "sp_before_sparen" will
> ensure a
> > single space is always added between a control statement and the
> > parenthesis.
> >
> > Thanks,
> > Michael
> >
> > On 11/16/2021 6:05 PM, Pedro Falcato wrote:
> > > Hi,
> > >
> > > RE: Uncrustify's configuration
> > >
> > > I had made a few changes to the config file, when trying out
> > Uncrustify,
> > > a few months ago, on Mike Kinney's suggestion. In my experience,
> > the new
> > > config file reflects the edk2 coding style better.
> > >
> > > In particular, it adds spaces between things like 'if' and the
> > > parenthesis, and better aligns functions' parameters.
> > > Feel free to check out the diff:
> > > https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>
> > > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>>.
> > > There's a bit of diff noise due to it being based on
> > >
> > https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>
> > >
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>>
> but
> >
> > > diffed on
> > >
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> >
> >
> > >
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> >>.
> > >
> > > Thanks,
> > > Pedro
> > >
> > > On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki
> > > <mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>
> > <mailto:mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>>> wrote:
> > >
> > > I think that makes sense. I will look into it further and let
> > you know
> > > if there's any downsides found.
> > >
> > > On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> > > > Could we add this feature to the Uncrustify CI Plugin?
> > > >
> > > > Mike
> > > >
> > > >> -----Original Message-----
> > > >> From: Michael Kubacki <mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>
> > > <mailto:mikuback@linux.microsoft.com
> > <mailto:mikuback@linux.microsoft.com>>>
> > > >> Sent: Tuesday, November 16, 2021 10:54 AM
> > > >> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>; Kinney,
> > > Michael D <michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>
> > > <mailto:michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>>>
> > > >> Subject: Re: [edk2-devel] Uncrustify configuration file
> and
> > > file/function templates
> > > >>
> > > >> I would prefer to have a single version of the file if
> > possible to
> > > >> reduce synchronization issues across the two copies.
> > > >>
> > > >> It seems that a CI plugin to read the contents of the
> > template
> > > files and
> > > >> search incoming code for that text wouldn't be too
> > difficult to
> > > add as a
> > > >> new plugin.
> > > >>
> > > >> Thanks,
> > > >> Michael
> > > >>
> > > >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> > > >>> Hi Michael,
> > > >>>
> > > >>> Should we have 2 versions of the config file?
> > > >>>
> > > >>> One used by automation tools such as CI and git hooks
> > that do
> > > not use the
> > > >>> templates.
> > > >>>
> > > >>> And another one that a developer can optionally use that
> > will
> > > add the
> > > >>> templates for missing file/function headers that the
> > developer
> > > then needs
> > > >>> to fill out.
> > > >>>
> > > >>> One concern I have about the templates is if they get
> > used but
> > > a developer
> > > >>> does not fill in the missing information. It would be
> > best if
> > > a CI check
> > > >>> rejects a file/function header that has not been filled
> in.
> > > >>>
> > > >>> Mike
> > > >>>
> > > >>>> -----Original Message-----
> > > >>>> From: devel@edk2.groups.io
> > <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io
> > <mailto:devel@edk2.groups.io>>
> > > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>> On
> > Behalf Of
> > > Michael Kubacki
> > > >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> > > >>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>;
> > > Kinney, Michael D <michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>
> > > <mailto:michael.d.kinney@intel.com
> > <mailto:michael.d.kinney@intel.com>>>
> > > >>>> Subject: Re: [edk2-devel] Uncrustify configuration file
> and
> > > file/function templates
> > > >>>>
> > > >>>> Hi Mike,
> > > >>>>
> > > >>>> Those were just disabled because I typically run a
> separate
> > > invocation
> > > >>>> of Uncrustify with them enabled to isolate code which
> > is missing
> > > >>>> file/function headers. My thought was the templates are
> > > helpful but we
> > > >>>> would need to individually identify where they are
> > placed to
> > > file TCBZs
> > > >>>> for maintainers to replace the template with the actual
> > > information.
> > > >>>>
> > > >>>> In some of my previous poc branches (like
> > > >>>>
> > >
> >
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> > <
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>
> > >
> > <
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers <
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> >>),
> > > I
> > > >>>> also pushed a branch with those results.
> > > >>>>
> > > >>>> So I do think we would want them enabled in the final
> > config
> > > file. We
> > > >>>> can also review the contents of the templates in the
> future
> > > patch series
> > > >>>> to see if any changes are recommended.
> > > >>>>
> > > >>>> I prefer using a .uncrustify directory to help group
> > related
> > > collateral
> > > >>>> but I don't have a strong opinion there.
> > > >>>>
> > > >>>> Thanks,
> > > >>>> Michael
> > > >>>>
> > > >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> > > >>>>> Hi Michael,
> > > >>>>>
> > > >>>>> In your POC branch
> > > (https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>
> > > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> > <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>>), I see
> the
> > > >>>>> uncrustify.cfg configuration file in the root.
> > > >>>>>
> > > >>>>>
> > >
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>
> > >
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>>
> > > >>>>>
> > > >>>>> However, in your Wiki, you provide examples where this
> > > configuration file is in an
> > > >>>>> .uncrustify directory
> > > >>>>>
> > > >>>>>
> > >
> >
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> > <
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >
> > >
> > <
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> <
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >>
> > > >>>>>
> > > >>>>> The uncrustify.cfg files also contains commented out
> > settings
> > > for the file header
> > > >>>>> and function header templates.
> > > >>>>>
> > > >>>>> # cmt_insert_file_header =
> > > default_file_header.txt
> > > >>>>> # cmt_insert_func_header =
> > > default_function_header.txt
> > > >>>>>
> > > >>>>> Are these disabled on purpose?
> > > >>>>>
> > > >>>>> Do we want to enable them? If so, should the
> uncrustify
> > > configuration file
> > > >>>>> and the templates go into a .uncrustify directory?
> > > >>>>>
> > > >>>>> Thanks,
> > > >>>>>
> > > >>>>> Mike
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > --
> > > Pedro Falcato
> > >
> >
> >
> >
> > --
> > Pedro Falcato
>
--
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 24070 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-11-17 1:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-16 17:16 Uncrustify configuration file and file/function templates Michael D Kinney
2021-11-16 18:25 ` [edk2-devel] " Michael Kubacki
2021-11-16 18:31 ` Michael D Kinney
2021-11-16 18:53 ` Michael Kubacki
2021-11-16 19:18 ` Michael D Kinney
2021-11-16 19:26 ` Michael Kubacki
2021-11-16 23:05 ` Pedro Falcato
2021-11-16 23:20 ` Michael Kubacki
2021-11-16 23:30 ` Pedro Falcato
2021-11-16 23:54 ` Michael Kubacki
2021-11-17 0:32 ` Michael D Kinney
2021-11-17 1:46 ` Pedro Falcato
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox