qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Stitcher <astitc...@redhat.com>
Subject Re: svn commit: r1424091 [1/9] - in /qpid/trunk/qpid/cpp/src: ./ qpid/legacystore/ qpid/legacystore/jrnl/
Date Thu, 20 Dec 2012 00:28:33 GMT
I've also noticed a couple of things about this "new" code:

* It has a different convention for header files from qpid (in most
places) using .hpp rather than plain .h: I'm not sure this is worth
fixing.

* The header guards it uses are inadvisable:
The pattern used is _filename_ this is bad for two reason:

1) The name isn't scope to any directory so there could conceivably turn
out to be multiple header files in different places with the same guard.

2) using an initial '_' is bad. As long as the first character is lower
case it is legal, but if you were to make the next character uppercase
or another '_' then it isn't

At the very least the header guards should all be changed to include the
scope of the directory hierarchy they are in, and whilst fixing that 2.
might as well be addressed at teh same time.

The recommended (but patchily used) standard would be
QPID_LEGACY_STORE_...

Andrew

On Wed, 2012-12-19 at 17:08 -0500, Chuck Rolke wrote:
> I'll remove the copyright notices. Thanks for the reference link.
> 
> -Chuck
> 
> ----- Original Message -----
> > From: "Robbie Gemmell" <robbie.gemmell@gmail.com>
> > To: dev@qpid.apache.org
> > Sent: Wednesday, December 19, 2012 4:19:08 PM
> > Subject: Re: svn commit: r1424091 [1/9] - in /qpid/trunk/qpid/cpp/src: ./ qpid/legacystore/
qpid/legacystore/jrnl/
> > 
> > I noticed many of the new files have copyright notices. From previous
> > reading I had the impression that is frowned upon for directly
> > contributed
> > code...might be worth a read of:
> > http://apache.org/legal/src-headers.html#headers
> > 
> > Robbie
> > 
> > On 19 December 2012 20:34, <chug@apache.org> wrote:
> > 
> > > Author: chug
> > > Date: Wed Dec 19 20:34:56 2012
> > > New Revision: 1424091
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1424091&view=rev
> > > Log:
> > > QPID-1726 ASF licensed, QPID hosted store
> > > This checkin lands the store mission code. Tests to follow.
> > >
> > > Review at https://reviews.apache.org/r/8556
> > >
> > 
> > <snip>
> > 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> For additional commands, e-mail: dev-help@qpid.apache.org
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Mime
View raw message