Re: [Development] API review and API quality

2016-01-27 Thread Marc Mutz
On Monday 18 January 2016 15:48:54 Andreas Hartmetz wrote:
> Everything I have written about API applies to documentation as well. It 
> seems like whatever the implementor writes is accepted and that is that. 
> AFAIU that was not exactly how it used to be done at Trolltech when it 
> was still called Trolltech?

+1

To wit: https://codereview.qt-project.org/146112 (staged while being -1'ed for 
the docs (and commit message)).

/me shakes head

-- 
Marc Mutz  | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] API review and API quality

2016-01-19 Thread 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'
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] API review and API quality

2016-01-18 Thread Andreas Hartmetz
On Montag, 18. Januar 2016 17:41:38 CET 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
> 

It is a mixed blessing that I didn't remember that :P
My impression is still that API is not taken as seriously as more 
technical issues in the typical review, by both reviewers and submitters 
of changes.

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

Oh! That is nice. Was that announced somewhere? I've mentioned vague 
intentions to write the e-mail about API reviews to some coworkers who 
didn't seem to know about that, and I also didn't find anything in a 
quick search of the mailing list archive and the wiki now.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] API review and API quality

2016-01-18 Thread Oswald Buddenhagen
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.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] API review and API quality

2016-01-18 Thread Curtis Mitch
I don't really understand the question you're asking. :D

> -Original Message-
> From: Development [mailto:development-boun...@qt-project.org] On Behalf Of
> Andreas Hartmetz
> Sent: Monday, 18 January 2016 3:49 PM
> To: qt-dev 
> Subject: [Development] API review and API quality
> 
[snip]
> Nowadays, API not contributed by TQtC is not-really-reviewed on Gerrit.

Can you share an example of this?

> Everything I have written about API applies to documentation as well. It
> seems like whatever the implementor writes is accepted and that is that.
> AFAIU that was not exactly how it used to be done at Trolltech when it was
> still called Trolltech?

Here as well? 

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


[Development] API review and API quality

2016-01-18 Thread Andreas Hartmetz
Hello,

Due to a recent problem I had with an API addition (solved while writing 
the E-Mail about it, the rubber duck technique worked!) I noticed, not 
for the first time, something missing...

the Trolltech API review process.
The thing that ensured that (almost) all new API made sense to humans 
semi-informed about the subject matter the API deals with.
I don't know how it was done - I recall some rumors about people getting 
into a room and reviewing API on a projector or something.

Nowadays, API not contributed by TQtC is not-really-reviewed on Gerrit. 
Gerrit is somehow much more detail-oriented, and criticizing "too 
subjective" stuff is frowned upon. There's a fine line between annoying 
people for no good enough reason and being too lenient and letting bad 
code / API slip through.
For API, due to its somewhat subjective nature, I would argue that the 
level of strictness required to achieve a good result is already more 
strict than what is perceived as annoying people for no good reason. It 
doesn't help that the strictest and most nitpicky reviewers generally 
care the least about API.

So yeah, I came with a problem and no good suggestion for a solution.
I would be fine with TQtC doing API reviews at some point well before 
release and telling everyone the results in time to improve their code 
additions for that release, but that's just my opinion.
I suspect that a room full of "dude, that API sucks, you can't ship 
that" makes more of an impression on perpetrators of subpar API than one 
or two -1 for "only" soft reasons on Gerrit. At an API review meeting, 
there *will* be a sufficiently large number people to achieve that 
effect. On Gerrit, well, most people prefer doing something else, like 
micro-optimizing something. Nobody can argue with contributions like 
that. But they are perhaps not as important as ensuring API quality.

Everything I have written about API applies to documentation as well. It 
seems like whatever the implementor writes is accepted and that is that. 
AFAIU that was not exactly how it used to be done at Trolltech when it 
was still called Trolltech?

tl;dr: API quality is not something people work on casually and 
voluntarily, it seems like it needs more / more suitable process to 
achieve.
(Also, nobody likes process.)
Now what?

Cheers,
Andreas
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development