[Development] API review and API quality

[André Pönitz]

On Mon, Jan 18, 2016 at 05:41:38PM +0100, Oswald Buddenhagen wrote:
> On Mon, Jan 18, 2016 at 03:48:54PM +0100, Andreas Hartmetz wrote:
> > Gerrit is somehow much more detail-oriented, and criticizing "too 
> > subjective" stuff is frowned upon.
> >
> anyone who complains about such aspects of a review clearly didn't quite
> get https://wiki.qt.io/Qt_Contribution_Guidelines -
> 
> "Our API Design Principles aim for perfection."
> 
> it's also point 1.2 of https://wiki.qt.io/Commit_Policy
> 
> > Now what?
> > 
> we actually already decided some months ago that TQtC sets up an api
> review task force at pre-release time.
> we've yet to see how that will work out in practice in the longer run.

Right, *that* jury is still out.

But to be honest, I am pessimistic here, beyond my usual self.


1. There are always good reasons to not touch code (e.g. for fixing
"wrong" API) "just before the release". This can be as mundane as "CI
got the phase of the moon wrong", up to "I am busy" or "I don't want to
start yet another discussion about the color of bike sheds" producing a
bias towards leaving stuff as-is, especially if it looks only mildly
wrong. This is in stark contrast to the setup Andreas refers to when
you'd *first* get a virtual nod from Mr B on the correct use of names
and *then* started to create a patch.

2. Any two out of 200+ approvers can add stuff but there is practically
no means to fix mistakes due to some compatibility promises once a minor
release is out. Even if one firmly closes eyes and imagines a world where
all those people agreed at least on basic ideas like "API consistency is
an asset" or "experiments are better done in toy projects, not in the
library", have no personal agenda etc etc there is still a big chance
that (a) real mistakes happen, and more importantly (b) even those
benevolent people would still produce inconsistent result because there's
lways a subjective component when applying rules, no matter how strict
they are.


Solutions?

Obviously no silver bullet, but:

- API review task force before releases is a step forward to help to
  iron out obvious mistakes, but it's not a full solution as happens
  too late in the process and "running out of time" works against it.
  API review needs to block before things happen. We already have bots
  on gerrit for things like string changes, having something similar
  for stuff that'll fall under BC/SC promises doesn't seem infeasible.

- There must be a means to stop people handling core parts of the
  project as playground for self-realisation.

- It would be helpful if there was way to undo mistakes before
  the next major release happens (and that should not be the
  equivalent of dropping whole modules).

Andre'