* .hpp vs .h: There's no better time to fix this than now before the test code leaks in and starts making the effort harder.
* Header include guards: It's "new" code so let's make this right, too. Having possibly ambiguous definitions is not good; to fix on separate commit. -Chuck ----- Original Message ----- > From: "Andrew Stitcher" <astitc...@redhat.com> > To: dev@qpid.apache.org > Sent: Wednesday, December 19, 2012 7:28:33 PM > Subject: Re: svn commit: r1424091 [1/9] - in /qpid/trunk/qpid/cpp/src: ./ > qpid/legacystore/ qpid/legacystore/jrnl/ > > 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.gemm...@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, <c...@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-unsubscr...@qpid.apache.org > > For additional commands, e-mail: dev-h...@qpid.apache.org > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org > For additional commands, e-mail: dev-h...@qpid.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
