Re: rebase / STL set patch

[Robert Collins <rbcollins at cygwin dot com>]

On Thu, 2002-08-29 at 02:36, Jason Tishler wrote:
> Rob,
> 
> On Thu, Aug 29, 2002 at 01:51:55AM +1000, Robert Collins wrote:
> > On Thu, 2002-08-29 at 01:44, Jason Tishler wrote:
> > > Huh?  Do you mean the RebaseConfigParser::parseFoo diffs?
> > 
> > The missing return true from the parser submethods. status=foo(), and
> > foo() didn't return a value.
> 
> Oops on my part -- I'm actually surprised that the compiler didn't catch
> these...

It does, as soon as you turn -Wall on :}. First thing I did when it
didn't work OOTB was to do that.
 
> > * follow the GNU guidelines where they don't contradict what I've just
> > said.
> 
> What about the following C++-ish stuff not (really) covered by the GNU
> coding conventions?
> 
>     o constructor initialization formating:
>     o whitespace formatting:

as per indent. I'm open to a real c++ formatter, but still haven't
evaluated the ones proposed when I discussed this a while ago.

>     o method formatting (i.e., capitalization):
> 
>       bool parseFreeList (...);
>            ^
>       or
> 
>       bool ParseFreeList (...);
>            ^

My preference:
ClassFooToo::methodFooBar

 
>     o data member formatting:

ClassFooToo::memberFoo

>     o etc?

The only other thing is class hierarchy naming:
(for single inheritance only)

ParentClass
ParentClassChild
ParentClassChildTwo

i.e. RebaseBuilder, RebaseBuilderMemory.

However: this last one is *purely* a rule of thumb, and I'm happy with
your class names.

Rob

Attachment: signature.asc
Description: This is a digitally signed message part