Proposal: KDE CVS Commit Policy

[Cornelius Schumacher]

On Sunday 30 March 2003 21:55, Dirk Mueller wrote:
> On Son, 30 Mär 2003, Cornelius Schumacher wrote:
> >  1. Think before committing.
>
> Is this a policy? If yes, what does it say? What does it require to
> prove that I think before I commit?

It's more an advice than a policy and it might sound trivial, but I 
think it's important that people think about the consequences of their 
commits before things get broken. Of course you can't prove this, but 
you might be able to measure it by the amount of complaints a commit 
gets ;-)

> KDE CVS has been traditionally very
> bad, and I can sum up some exisiting problems, which in turn, when
> you negate them, give a useful policy:

Ok, I will try to do that and include them in the document. I will post 
a new version with a separate mail.

> 1. Lack of communication. The maintainer does not know about the
> commits, nor has seen the patch before. Bigger commits usually
> contain therefore essential flaws.

That's basically what Neil proposed as replacement to point 6.

> 2. Lack of reviews before committing. Public API extensions and
> modification is rarely reviewed or discussed before committing. Hence
> we end up with major compatibility breakage in every major release.
> 3rd party application vendors find this a big annoyance.

Agreed, reviews of changes of public APIs are important. I will add that 
to the list.

>    Furthermore, big api extensions are usually done by people with
> very low experience. They don't implement maintainable interfaces,
> which includes simple thinks like d pointers, but extends to
> missing/too many virtuals, wrong inheritance, incorrect method
> naming, awkward API, terrible reusability and overfeaturism of little
> details and behaviour- modifications that can easily be implemented
> in a 20 lines of code additional layer that is much easier to grasp
> for the application developer. KExtendedSocket being a classic
> example.
>    Furthermore, I often see newly committed code with undefined or
> untested memory management and ownership policy. For code that should
> be resusable, documentating and defining this is topmost priority.
>
>    Implementations of "nice to have" API without general proof of its
>    necessity or relation to the class where they're implemented in
>    (KApplication::randomString. I think this 5 line method had about
> 25 bugs so far, besides being fatally missplaced in kdelibs. )
>
> 3. Lack of focus on maintainability, stability and security. Lack of
>    understanding about security implications and coding style to
> avoid the associated problems (shell quoting, buffer overflows,
> format string vulnerabilities, i18n bugs). Lack of common coding
> style (spaces vs tabs, indentation. curly bracket placement).

All these things are important, but I think we should separate them from 
a CVS Commit Policy because they rather address what should be 
committed than how it should be done.

> 9. Lack of maintenance of the stable release series. Often useful,
> smaller new features are not backported, while dangerous, untested
> "code cleanups" are immediately backported. Massively introduced
> regressions are not being cleaned up in the stable branch and end up
> in a release (I can't remember a single KDE release where kio_http's
> proxy support wasn't flawed in one or another way).

This is also an important point which should be clarified by a policy 
different from this one.

> 10. Lack of direction. Several developers refuse to accept guidance
>    by more experienced contributors, expressed in irrational and
> provocative ways ("release dictator", "maintainer", "if you don't do
> it my way I'll fork KDE").

Hmm, that's a very difficult issue. I don't think we will ever be able 
to put this into a reasonable policy. We don't have a way to define who 
is experienced or competent to give guidance. We can only recognize 
when advice from somebody actually is accepted by others, but there is 
no way we can enforce this.

> 11. Lack of acceptance of release schedules ("I'm allowed to commit
> my stuff when I think its ready". "Do what you want, I create my own
> branch and work in there". "Hey, CVS is for everybody, so don't
> interfere with my time schedule").

That's meant to be covered by point 5.

-- 
Cornelius Schumacher <schumacher@kde.org>