[Freeciv-Dev] Re: commit early, commit often (was: Submit patch again?)

[Kevin Brown <kevin@xxxxxxxxxxxxxx>]

Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Aug 17, 2001 at 09:27:13AM -0700, Kevin Brown wrote:

[...]

> > But "solid technical reasons" still covers quite a lot of ground.
> > Consistent use of available macros and functions is a good example,
> > especially in light of some of the other discussion going on at the
> > moment.  :-)
> 
> Mhh yes. But before a patch is rejected or the author asked for
> changes for such a reason there has to by a guide line. It may be good
> to start one when there is a conclusion about the macro-vs-function
> and the normal-vs-real-proper-tile topics.

Perhaps.  The guidelines, like anything else, are going to evolve over
time.  For instance, this bit about normal-vs-real-vs-proper tiles
wasn't even an issue until we started talking about generalizing the
code to deal with arbitrary map topologies.

> Never I had in mind a complete rejection. Either the author or the
> maintainer (if the patch is "good" and the author won't do it) has to
> do the cleanup.

OK, then we're basically in agreement on this.  Cool.

> > I'm not sure I follow.  If *all* of the source is run through indent
> > like this, then I can see a huge set of diffs being generated once,
> > but afterwards?
> 
> You are right. However nobody has the courage to do so. At least that
> was the answer I got.

LOL!  :-)

This is the PERFECT application of CVS branching.  You create an
"unindented" branch, indent the working branch, and any patches that
get submitted against the old "unindented" source you apply against
the "unindented" branch, then run indent against the "unindented"
branch, then diff against the "indented" source of the same rev level,
then apply the resulting diff against the "indented" working branch
(which is what everyone else sees).  Keep both in lockstep (as much as
possible, anyway) until nobody's submitting patches against the
unindented source.

You can test the indented version upon initial creation versus the
unindented version by compiling it up and seeing if it breaks
anything, prior to actually making the indented version the current
working version.  In fact, you could probably compare the generated
code: there should be no difference except perhaps in the ELF headers.

It's a little bit of work but I don't see much risk in it, myself.

> > Only to the extent that you can revert to a previous version.  Not the
> > same thing at all.  Oh, you could get diffs between versions to
> > isolate the bugfix patches and reapply them, but that's quite a bit of
> > work
> 
> I think this is sufficient.

Well, yeah, it is, if it weren't likely to be A LOT more work.  Why go
through the hassle when maintaining two branches is a lot easier?

> > , and as such would be viewed as "risk" by the CVS maintainers,
> > which would make them more reluctant to commit patches in general.
> 
> This would the same situation as now.

Right!  But the current situation is EXACTLY what we're trying to
correct, yes?

> You assume that there must always a tree which is stable. Is this
> really necessary? Let me define what kind of stableness I have in
> mind: at all time the tree has to compile without errors (except
> "exotic" parts like vms, amiga or beos) and a basic games (all ai and
> 2 humans plus ais) must be possible. There may be a chance that the
> recently introduced feature will only 90 out of 100 times. This is
> ok. It is also ok that the changes a feature made to the general code
> base breaks 5 out of 100 times.

Well, even though this is a game and thus isn't mission critical to
most people :-), we still want people to play it, yes?  Which means
that we want them to be able to get their hands on something rock
solid.  I don't know about you, but I don't like it when ANY program
I'm using crashes or otherwise misbehaves, unless I'm interested in
testing out new features or something.

So yes, we *do* need a stable tree, if only to satisfy those people
that really don't want to be on the bleeding edge.  No matter how hard
we try, we're going to end up with bugs in major releases.  We need to
be able to address those bugs without simultaneously subjecting the
user community to a bunch of (relatively) untested patches.  That's a
lot more difficult to do if you're only maintaining one tree.

Previously, the focus has been on keeping Freeciv solid.  I don't
think we should sacrifice that goal lightly.  Maintaining a
development branch and a stable branch gives us the ability to keep
Freeciv as solid as we have been while at the same time allowing us to
develop it at a more rapid pace.  Maintaining a single branch means
that, no matter how you do it, *some* kind of compromise has to be
made: either you don't commit stuff to CVS as readily, or you
sacrifice stability.

It's not like maintaining two branches like that is going to be that
much extra work or anything, especially if you get someone to
volunteer for stable branch maintenance.

> > Even so, you still haven't addressed my main concern, namely that the
> > use of a single tree will result in a very conservative application of
> > patches: if I'm a CVS maintainer and I know that the application of a
> > feature patch may make a lot of work for me later if I have to back it
> > out, then I'm going to be a lot more reluctant to commit it.
> 
> Not if the patch worked in civserver or another developer tested
> it. 

There's no substitute for community testing!  A single developer's
testing can easily miss something that would be caught through testing
by many.  This is one of the major strengths of open source!

Remember that changes to the server, particularly significant ones,
often require changes to the client.  We depend heavily on the rest of
the world to test this stuff.

> So serveral kinds of feedback is needed:
>  - general feedback about the idea
>  - feedback about the style and correctness
>  - feedback about actually trying out the patch

Well, you still want that, but none of it is a substitute for knowing
that you can back out major changes without too much effort.  I'd
argue that these are orthogonal.

> The question is when get you the feedback: before the cvs commit or
> after it or after the commit to the first branch but before the secon
> one?

Unfortunately, *when* you get the feedback isn't always going to be up
to you.  As experience with this list shows, patch submitters often
won't get any feedback on their patches at all.  So this, combined
with more aggressive CVS commits, means that much of the feedback
you'll get will be after the patch gets applied to CVS.  Which
increases the chance that you'll have to back out the patch after it's
already been committed.  It's easier to deal with issues like this if
you don't have to worry about affecting stable, bugfixed code.


-- 
Kevin Brown                                           kevin@xxxxxxxxxxxxxx

    It's really hard to define what "unexpected behavior" means when you're
                       talking about Windows.