This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH] Bigger Chooser 2


> 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.
>

The 31k of the patch implementing the bigger chooser is not decomposable into
smaller patches.  There is not always a path from A to B that can or should be
divisible.

> > 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.
>

How am I going to do that?  The changes are made on my end.  I can't "un-make"
them and "re-bring" them in, can I?

> >   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.

No, it isn't.  I'd rather have made small patches, sent them in, and have them
reviewed and committed by the time I sent in the next small patch.  That hasn't
been the way things have worked, at least until the last few weeks anyway.  So
any small patch I would have sent would have languished, the next one I sent
would have to have been diffed against HEAD and hence would contain the previous
patch anyway, lather, rinse, repeat.  The only option I see would be to submit a
patch and wait until it got committed before touching the code again, and I've
able to devote too little time to setup as it is.

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

As is mine.  As is all of ours.  I would think have a single large patch as
opposed to many equivalent small patches (assuming it's possible for given large
patch) would induce less reviewing overhead and have no real effect on actual
reviewing effort.  Particularly in a case like this, where it's one primary
patch and one "fix/improve a bunch of little things" patch.

> > 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.
>

I could, but since it is orthogonal, what's the difference?

--
Gary R. Van Sickle
Brewer.  Patriot.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]