Proposal: KDE CVS Commit Policy

[Dirk Mueller]

On Son, 30 M=E4r 2003, Cornelius Schumacher wrote:

>  1. Think before committing.

Is this a policy? If yes, what does it say? What does it require to prove=
=20
that I think before I commit?

Obviously everybody thinks before committing. Just sometimes they don't=20
think *enough*, or do their thinking on wrong assumptions (or, being humans=
,=20
make mistakes).=20

I personally don't think your suggestion is a useful policy. It neither=20
documents the currently established common sense, no improves the situation=
.=20

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

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

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

   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,=20
   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=20
   developer. KExtendedSocket being a classic example.=20
   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).=20

4. Big code reorganizations without prior discussion, including massive
   renaming of files and moving around in CVS modules (see ksplashml
   for example lately. ). CVS-unfriendly usage of CVS: import in a separate
   directory, which will make later tracking of history almost impossible
   when the directory will be copied over to kdebase/ksplash

5. So called "fixes" which have obviously not been regression tested.=20
   The kdelibs/tests check applications most often don't even compile
   after such a commit.=20

6. Patches for supposed fixing of problems which have not been completely
   investigated or understood by the committer. ("I don't know why it
   crashes, but when I do this, it does not crash anymore." or
   "I don't know what happens, but till somebody finds out, I'm disabling
   it". or "I'm not completely sure if thats right, but at least it=20
   works for me.")

7. Constant build breakages. Continuous fiddling with makefiles, with
   include paths, addition of uic or moc generated files to CVS,=20
   lack of committing necessary files (new header files, new icons etc).=20

   Introduction of GNUisms in implementations, ugly shell hacks in
   install-local: and similiar targets in the Makefile.am=20
   (result of lack of understanding of the KDE build system).=20

8. Lack of descriptive log entries. Often bugreports are not mentioned=20
   in the commit log, or are not closed at the same time. the log
   entries usually just describe what can be read from the diff, but don't
   explain the _how_ or _why_.=20

9. Lack of maintenance of the stable release series. Often useful, smaller
   new features are not backported, while dangerous, untested "code=20
   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).=20

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

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=20
   interfere with my time schedule").=20

12. Lack of willingness to change the situation. Most often people don't
   care about even massive breakages or obvious introduced regressions
   as long as they still find a way to work around it (or revert to an
   older version until somebody else is stupid enough to fix it).=20


--=20
Dirk