Re: rebase / STL set patch

[Jason Tishler <jason at tishler dot net>]

Rob,

On Wed, Aug 28, 2002 at 06:17:04PM +1000, Robert Collins wrote:
> Neato. I've had a look-see.

Thanks for your time.

> I attach an updated version.

make clobber -- for the bandwidth challenged, next time?

> There were a few logic flaws that made it not work for me.

Huh?  Do you mean the RebaseConfigParser::parseFoo diffs?  I couldn't
find any other likely candidates.  Nevertheless, I don't see why it
didn't work before and does after your changes.  BTW, the handling of
parsing errors is very weak right now -- I have been meaning to add some
"FIXMEs."

Also, thanks for finding the missing virtual destructor.

> A bit of feedback... I had to violate the Free/UsedList abstraction
> layer to do the memory dumping. This is because they aren't exposing
> iterators themselves.

I can easily expose the iterators, if necessary.

> [snip] 
> This uses the STL containers themselves to implement the containers -
> if you do need custom container behaviour, then the current two-layer
> approach makes more sense

It's not apparent from the code that you reviewed, but I do.

> (but I'd still implement the RebaseState class).

OK.

> As I mentioned in my earlier emails though,

I didn't forget, but...

> it actually makes sense to have RebaseState inherit from
> RebaseBuilder, and implement the building interface itself - because
> it's already decoupled from the storage mechanism.

I couldn't find the right abstraction to implement this interface.  Now
that you suggested RebaseState it's obvious.

> There will a few minor things to change at that point to meet the
> setup coding conventions. (header file inclusion orders and the like -
> nothing major).

I would prefer to deal with these minor points sooner rather than later.
Are the setup coding conventions documented anywhere?  I tried to find
them at:

    http://sources.redhat.com/cygwin-apps/setup.html

Thanks again for your time.

Jason