[Freeciv-Dev] Re: AI - cleaning (Was: Re: [UPDATE] Corecleanup_08 patch)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Oct 10, 2001 at 12:31:43AM -0400, Ross W. Wetmore wrote:
> At 06:12 PM 01/10/09 +0100, Gregory Berkolaiko wrote:
> > --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> >> On Tue, Oct 09, 2001 at 03:32:15AM -0400, Ross W. Wetmore wrote:
> >> > At 05:44 PM 01/10/05 +0100, Gregory Berkolaiko wrote:
> >> >
> >> > I really don't see this changing without some major shakeup in the
> >> way 
> >> > Freeciv maintainers handle things. It needs to move from a despotic 
> >> > "Boss" mode to a state where there is a Magna Carta document or Code
> >> of 
> >> > Laws that sets out appropriate rules and responsibilities for both
> >> > submitters *and* maintainers. Trent pointed out a number of
> >> contributors
> >> > whose names have been blacklisted and scrubbed from Freeciv, or whose
> >> > code has been rejected only to reappear a little later in buggy
> >> mutated
> >> > form. This is *very* unprofessional, but typical of an immature
> >> system.
> >> 
> >> It would be helpful if you can come up with a list of what kind of
> >> things such a Magna Carta should contain.
> >
> >yes please
> 
> You can start with a basic Golden Rule.
> 
> Its relevance takes many forms ...
> 
> Someone who puts together a patch and spends the time and energy to
> get it working deserves to have the work treated with a modicum of
> respect and professional integrity. There are code reviews that 
> constructively improve the code and 

> there are inane suggestions or riders that seek to use the work to
> foster private agendas and petty needs.

I admit I have done this (suggest other changes which aren't necessary
for a particular patch). I haven't done this because of personal goal
but to improve freeciv as a whole. Sometimes such suggestions succeed
(Paul and the ARRAY_SIZE patch) sometimes not (settler_power). I don't
make the inclusion of a patch depending on the implementation of
suggested idea. And yes I didn't make this clear in the past. I hoped
that the people do the extra work if they think that it is needed. If
they didn't after some time the item goes to the todo list and I apply
the base patch.

I think it is good that people are aware of other non-local issues and
just don't patch the source in one place.

> And there are people who simply use or abuse other's work with 
> neither proper credit nor attention to the author's input or wishes.
> 
> Someone who submits a patch should provide documentation, install
> and test instructions and reasonable aids to make it as easy as 
> possible for others to come up to speed to give it a proper review. 
> They should also respect both the existing coding standards and goals 
> of the Freeciv, as well as the suggestions of others in how better to 
> meet them. If reviewer's are following the previous good practices, 
> then their comments are generally for a purpose and worth a good look.
> 
> Review patches based on their stated goals and functions. Decide
> early in the process if they are compatible and do proper thorough
> reviews if they are. 

> The concept of having people resubmit a patch repeatedly with one
> new trivial change each day because that is the level of effort
> reviewers are willing to make is both inefficient and petty,
> especially petty when it used as a threat.

You can't expect that the reviewer can catch all issues in one
pass. If you have proof read some paper you will know this.

> Treat patches as a unit of work and apply them as finally approved
> or not at all without *any* further change. Changes can always be
> applied in subsequent patches if the need arises, but in general
> such followup patches unless indicated during the review denote a
> failure of the reviewers and review process.
> 
> In short ...
> 
> Follow basic rules of common sense that provide both the submitter 
> and those responsible for maintaining Freeciv with incentive to 
> participate in the process and move it quickly and effectively to
> worthwhile conclusions.
> 
> A proper review process with prereqs and appropriate conditions for
> advancing to the next stage is a useful way to keep the process 
> orderly and uniformly fair to all involved.
> 
> The author should always have the deciding say over the code in the
> patch. 
> 
> The maintainer should always have the deciding say in whether it is 
> applied to CVS or not.
> 
> And make sure that people that are constantly saying "I won't allow
> <blort>" where <blort> is the spontaneous excuse of the moment to
> show their petty power are quickly castrated or removed from any
> point where they can seriously impact the constructive processes.
> Egos and personal agendas need to be relegated to the development 
> dustbin. Concensus from discussion will quickly sort out the "I"s
> from the "us"es.
> 
> You can probably do a ten commandments or similar to frame the 
> appropriate deadly sins for developer and maintainer alike. I
> would suggest you take suggestions at large and pass the proposed
> flavours around for discussion and final vote if this is the
> way you choose to go.

I agree mostly. But what are the short rules? One rule which now came
across boldly is that "Maintainers should commit the patch of the
patch author unchanged". Others like "the patch author should provide
documentation" and "maintainers shouldn't use excuses with have no
base" are nice but IMHO aren't rules.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Using only the operating-system that came with your computer is just
  like only playing the demo-disc that came with your CD-player."