Re: [PATCH] Split protos.h into component-specific header files

On 24/08/2012 2:50 a.m., Kinkie wrote:
>> I've just taken a quick look through and can find nothing terribly
>> objectionable.
>>
>> Please do not add new sets of multiple empty lines.
>> * all the new headers are there with 2,3,4+ needless whitespace lines
>> around the #ifdef/#endif lines.
> We have a script for that (which I submit for inclusion). Please see
> attached remove-duplicate-empty-lines.pl

I know. But its better not to have them added in any new stuff only to
remove it again immediately in a followup patch.

Can this scripts task be merged as one of the output_filter's in
script/formater.pl please. That change can go in separate from these
rest of these polish patches - ie with nothing outside of formater.pl
touched by that commit.

If formater.pl could be made to drop empty lines at the begining of the
file (first lines only) that would be good too.

>
>> * please do not use double empty lines to mark special significance of a
>> block of code. If you need to emphasize anything like that it would be
>> better to add a comment in place of the second empty line and explain to
>> other dev why the block is separate.
> There is no such special meaning. One empty line will do, but also a
> comment would get the same effect.

Okay. Then the case I found being added by this were just more useless
double-lines being added by this patch.

>
>> Also, on the styling I'm an advocate of placing the copyright disclaimers
>> inside the pre-compiler .h #ifdef protection clause. That can greatly reduce
>> the size of precompiled temp files and saves the actual compiler from a
>> little bit of duplicate work skipping them. We have no consistency in the
>> code right now, so this is just a comment to keep in mind (or debate) not a
>> change request.
> As far as I can tell, GNU cpp removes comments during precompiling.
> But other compilers may act differently, so you may have a valid
> argument.
> Fortunately, we have a script for that (which I also submit for
> inclusion). Please see attached headers-copyright-adjust.pl .
> It can be piped with the whitespace-removal tool.

I have some other things I want this to do with the wrapper macros as
well. Will get back to you about that next week.

Amos
Received on Fri Aug 24 2012 - 02:19:24 MDT