Re: [PATCH] Sort alphabetically squid-specific header files

On 08/21/2012 05:07 AM, Kinkie wrote:
>> Please include the Perl script in the patch. It should be committed so
>> that others can use it when modifying files. Ideally, it should be
>> integrated with the automated source formatting scripts, of course.
>
> Included. About automatic formatting scripts, I leave that up to Amos.
> Honestly, I'm not sure it's worth it.

It is worth it if we want to enforce the order. Humans will miss
out-of-order includes during review process. This is something a program
does best.

What about reordering #includes in .h files? Why limit this to .cc?

Thank you,

Alex.

>> Please add a short #comment to the Perl script, to describe what it does.
> Done.
>
>> Please use "use strict" and "use warnings" in the Perl script.
> Done.
>
>> Consider adjusting the regex detecting an #include statement to handle
>> "#\s+include" cases (with whitespace between # and include), but we seem
>> to be using that syntax for system (and generated) files only for now,
>> so this is optional.
>
> Ok; leaving as is for maximum simplicity for now.
>
>>> The resulting tree passes the test-suite on my Kubuntu Precise.
>>> If it's OK, I'll merge the patch and add the sort-includes.pl tool in scripts/
>>
>> Please do not merge until the current build is fixed. Whatever method of
>> testing you are using, it does not apparently catch basic build errors
>> in common configurations, so we better take it one step at a time.
>
> The attached patch is up for review, as per standard process.
Received on Tue Aug 21 2012 - 15:11:00 MDT