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
