RE: [PATCH] Bigger Chooser 2

[Robert Collins <rbcollins at cygwin dot com>]

On Sat, 2003-03-29 at 19:30, Gary R. Van Sickle wrote:

> > Yes. I've never liked 100K patches, even when I do them.
> >
> 
> The source patch plus the new source file is only 50k total, at least 31k of
> which is solely bigger-chooser (primarily res.rc, proppage.{cc,h},
> propsheet.{cc,h}, Window.{cc,h}) and upon cursory examination simply is not
> divisible.  Significant changes were required to implement this functionality,
> and it just isn't something that can be checked in piecemeal and maintain a
> working HEAD.

Even so. There is *always* a path from A to B that doesn't break
functionality along the way. Thats what refactoring is all about. 

I do a cursory review of the patch, and IF I'm comfortable will consider
a all-in-one commit. But: don't get your hopes up.

> The other <19k is largely context around the one-liner OnActivate() changes you
> mentioned, OnInit() one-liners, and miscellaneous other one-liners.  I don't
> know if stripping these from the patch will result in a working or compilable
> patch or not.

Well, you can bring them in, one at a time, and with just a little care
nothing will break.

>   When I made these changes (OnActivate() intended to reduce the
> "next page specified/controlled in several places" issue), setup patches were
> not being reviewed and checked in in a timely manner, so I figured I'd do these
> all at the same time since that's what I'd end up having to submit anyway.

Well, thats your choice. Seriously, such commingling raises the bar to
having patches accepted. It makes my job harder. My bandwidth is scarce
enough as it is.

> And like I said, the new icon is just a bonus ;-).

It's also largely orthogonal. You could easily offer a separate patch
for that.

> So it's late, I'm babbling, wrap it up Gair.  Yes, there's two discernable
> "conceptual" patches here (well, three if you count the icon which I hope you're
> not counting):

The icon is a separate discussion.

> one's >31k of chooser enlargement, one's <19k of mostly
> "diff -pu" context.  Yes, I sympathize with the "one patch / one concept"
> philosophy (I think, don't quote me on that like I said it's late).  Yes, I have
> no desire to split this into two patches (which sounds like a rather large
> amount of unproductive work), and I don't see any size-related basis for doing
> so.

I will review quickly now..

If I find significant questions - I will be asking *someone* to extract
the various components of the patch.

Rob

-- 
GPG key available at: <http://users.bigpond.net.au/robertc/keys.txt>.

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