This is the mail archive of the
cygwin-apps@cygwin.com
mailing list for the Cygwin project.
Re: rebase / STL set patch
- From: Jason Tishler <jason at tishler dot net>
- To: cygwin-apps at cygwin dot com
- Date: Wed, 28 Aug 2002 11:44:36 -0400
- Subject: Re: rebase / STL set patch
- References: <20020630134832.GA1904@tishler.net><023001c22078$d26c9150$1800a8c0@LAPTOP> <20020701123502.GB1904@tishler.net><009001c220fd$4ff90610$1800a8c0@LAPTOP> <20020702212203.GC1776@tishler.net><039b01c22219$f796fef0$1800a8c0@LAPTOP> <20020731124402.GB1444@tishler.net><1028131377.2788.32.camel@lifelesswks> <20020827171043.GA508@tishler.net><1030522625.13255.440.camel@lifelesswks>
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