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

On 23.08.2012 03:24, Kinkie wrote:
> Hi all,
> the attached patch is the first part (of what will be 2, judging
> from the reduced side of protos.h) of my current refactoring effort
> aiming at splitting protos.h into more easily-manageable chunks.
>
> With this patch, the affected prototypes are moved close to their
> respective implementation file (e.g. prototypes for functions defined
> in ftp.cc are moved to the ftp.h file, creating it if it doesn't
> already exist). For the affected prototypes, linkage is also changed
> from c to c++. Include clauses and Makefile.am are adjusted
> accordingly.
>
> This patch has been build-tested (eCAP included), and
> farm-build-tested. It succeeds to the extent where trunk succeds.
> As there are no functional changes, I don't expect it to bring any
> functional problems.
>
> I'd like to commit to trunk, and to get some feedback before spending
> more time on this.
>
> Thanks

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

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.

Amos
Received on Thu Aug 23 2012 - 01:07:14 MDT