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

From: Kinkie <gkinkie_at_gmail.com>
Date: Tue, 21 Aug 2012 13:07:23 +0200

> 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.

> 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.

Thanks

-- 
    /kinkie

Received on Tue Aug 21 2012 - 11:07:30 MDT

This archive was generated by hypermail 2.2.0 : Wed Aug 22 2012 - 12:00:13 MDT