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
