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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Aug 16, 2001 at 10:10:39AM -0700, Kevin Brown wrote:
> Reinier Post <rp@xxxxxxxxxx> wrote:
> > On Wed, Aug 15, 2001 at 11:33:32PM -0700, Kevin Brown wrote:
> > > As a developer, feedback on a patch is CRITICAL.
> > 
> > OK.  (BTW, there's no need to argue that point.)
> 
> Ah, OK.  Wasn't sure, since patch feedback is sometimes hard to get
> here.  :-)
> 
> > OK, still it would help to give explicit instructions for testing.
> > "Start a game, turn off FOW, and move around the map a bit."
> > Dumb it down to the level of those who haven't seen the code.
> 
> Yeah, definitely agree here.  I also think the author should describe
> the patch (what it's good for, the technical details, etc.).
> Basically, all the things you'd expect from someone who's submitting a
> bug report and fix, since that's basically what it is.

I don't see a problem here except my own patches.

> > > > Another problem is that patches get lost or outdated too easily.
> > > 
> > > This is because they're not being committed to CVS!
> > 
> > Not sure.  If they were in CVS (on separate branches?)
> > their status might be even harder to follow.
> 
> The followups to this have been quite insightful.  I certainly don't
> think we should create a separate CVS branch for every patch!  But I
> think it makes sense to do so for the patches that appear to be very
> complicated and otherwise difficult to back out.  In other words, you
> create a separate branch for the high-risk patches, and then merge the
> branch back into the main trunk once it's been tested.
> 
> It's very important for the high-risk branch to be built and tested by
> the community.  Otherwise it'll stagnate.

IMHO it is hard to decide if a patch is high-risk or not. It should be
possible to minimize the risk by carefully reviewing and testing
it. It should be possible to turn the feature off if the patch is
controversially.

> 4.  All patches get a couple of days of review time before being
>     applied, but if nobody says anything about a patch, that
>     automatically means that it gets applied.

IMHO this shouldn't be measured with time but with approvals and
rejects. No formal system, just an email with yes or no (and a comment
in case of a no).

> 6.  On a periodic basis (say, three to four months), we declare a
>     feature freeze in the development branch, subject to whether or
>     not the code is in a suitable state for that.  New feature patches
>     can still be submitted, but they'll be queued up instead of being
>     applied against the tree.  Bugfixes get applied as usual.

Ack.

> 7.  Patches get applied to the appropriate places even if there are
>     trivial objections to them (such as style).  If someone has a
>     problem with the style of a patch, they can supply another patch
>     to fix it after it's been applied!  If the person responsible for
>     CVS commits has such a problem with the patch, then it's his
>     responsibility to fix it before applying it to the tree.  But a
>     patch should NOT EVER be rejected for trivial reasons!  The only
>     reason to reject a bugfix patch is if there is substantial
>     objection to it on this forum.  So people here should be given a
>     couple of days to comment on a patch before it gets applied
>     against CVS, unless it's an "emergency" bugfix against the stable
>     tree.

Nack. The submitter has to provide the style. I don't think it is
dis-encouraging if the patch sender gets a "we love to apply the patch
but there are style issues with it, please correct them" email.

> 8.  Under no circumstances should a patch be ignored.  Ignoring a
>     patch is the same thing as telling the submitter that his efforts
>     are unimportant, which says all sorts of bad things about the
>     project.

Ack.

> 9.  We need to remember that this is a game, not an operating system.
>     So it's actually okay to break things, even in the stable tree,
>     from time to time, as long as they get fixed in a timely fashion.
>     If a stable version ends up being fundamentally broken, we'll hear
>     about it soon enough!  People can revert back to a previous
>     version if they have to.  We don't want such breakage to happen
>     often because we don't want our reputation tarnished TOO much, but
>     neither should we be afraid of it to the point of paralysis.
> 
> 
> Thoughts?

I don't like the idea of two separate cvs branches. They just spread
the manpower more. It looks like a lot of people play via
civserver. Is it possible to patch servers at the civserver with
controversially patches? So players can test new ideas in an easy
way. Let the server print out at the start of the game something like:
"
  This is cvs version 2001-xx-yy with patch xyz version n applied
  This patch does:
     ....
  Please provide feedback to freeciv-dev or to the author <aslkdj@xxxxxxxxx>
"

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I do feel kind of sorry for Microsoft. Their attornies and marketing
  force must have tons of ulcers trying to figure out how to beat (not
  just co-exist with) a product that has no clearly defined (read
  suable) human owner, and that changes on an hourly basis like the
  sea changes the layout of the sand on a beach. Severely tough to
  fight something like that."
    -- David D.W. Downey at linux-kernel