From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney To: Daniel Jacobowitz Cc: Elena Zannoni , gdb-patches@sources.redhat.com Subject: Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS Date: Thu, 11 Oct 2001 17:43:00 -0000 Message-id: <3BC63C81.5000102@cygnus.com> References: <20011009123457.A28534@nevyn.them.org> <15302.12121.778853.77938@krustylu.cygnus.com> <20011011195450.A22256@nevyn.them.org> X-SW-Source: 2001-10/msg00157.html > > OK. Would you prefer I resubmit this patch broken up further, then? > I could do that. > > There's a double-edged sword here; every patch in this sequence except > for the hashing change is predicated on the previous patches. So while > I understand that breaking them up does make reviewing much easier, > with the current length of the patch review cycle, every time I > decompose this further I add two or three more days to its eventual > (hopeful) approval. I'm sure you can understand that it's a little > frustrating. Daniel, As you said, it is a double-edged sword. The other edge has a very unusual feature. Identify simple mechanical self contained changes and often go in as obvious. The review cycle goes down and can often be reduced to zero. While this can mean an increased workload for you as an individual it does dramatically reduce the work load for the entire GDB community. My reading of Elena's comment: > Yes, I looked ths over and it seems to work, except that I would really > prefer the change to printcmd.c split in two. The first bit to > rationalize that "if (func)..." code. This would have with it all > the indentation changes as well. The code as it is now doesn't really > make much sense. So, that looks a good change to me. But it has nothing > to do with the new macro. After that change is in, you can introduce > the macro in printcmd.c w/o having all the indent changes. > It also makes it easier to distinguish a no-op change (the macro) from > the other one. is that you're all approved. Your first commit fixes some messed up logic. It is a cleanup (but pretty obvious). It doesn't have anything to do with the (ULGH) macro. By keeping it separate it makes it possible to better isolate the breakage it could cause when we have to go back (in 6 months) to find a bug ;-) Your second commit is this new (ULGH) macro. The macro (ULGH) shouldn't break anything but it is however still a (ULGH) macro. Just include the extra tweeks you found. (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro so I'm biteing my tongue on this change :-) Andrew