[Development] Ameliorate API Reviewing (Was: On the effectiveness of time based releases)

[Alan Alpert]

On Mon, Feb 24, 2014 at 10:22 AM, Thiago Macieira
<thiago.macieira at intel.com> wrote:
> Em seg 24 fev 2014, às 19:00:28, Frederik Gladhorn escreveu:
>> > Some of those features then either caused regressions (... ok,
>> > that's what betas are for, although it may also be a symptom of poor
>> > testing in order to get the feature in), or are highly debatable
>> > from an API / technical point of view (which is even worse, because
>> > we can't change APIs or behaviour once we release).
>>
>> Yes, I feel we haven't reviewed new public API in a sensible way. Any
>> suggestions how to make this work? In the Nokia days (and earlier of
>> course)  there were long long sessions discussing API and perfecting it. In
>> a sense it's really hard to achieve the same over IRC and mail, but maybe
>> we should come up with a better way of dealing with this.
>
> I agree with the rest of what Frederik said, so I'm deleting it and focusing
> on the part above: API reviews.

Yes, that needs work. Which I think is mostly independent of the
branching/releasing discussion (on which, my opinion is that the
release team can decide - if I learned dev/stable/release at their
request I can learn anything ;) ).

> >From our release guidelines, "alpha" means we're releasing something for which
> the functionality will be reviewed. So API reviews done before an Alpha are to
> guarantee minimum alignment with Qt API guidelines. I've done several of those
> and I've seen others do the same, so it seems like we are doing it.
>
> However, since the point of an Alpha is to gather feedback on the
> functionality and API, it's not frozen in time. There's a lot of opportunity
> left to review it and modify the API. So a second round of comprehensive API
> review on any new things should be done after the alpha.
>
> Of course, this all depends on the size and complexity of the API. For smaller
> things, there probably won't be any controversy. For bigger things,
> stakeholder input is required even before merging into dev -- that was the
> case, for example, for QFileSelector.
>
> Finally, note that alpha is not a review of the *implementation*. That's what
> the beta is for. But even if that is done at the beta release, we still need
> to have good implementations. Sloppy code won't be accepted. So why can't we
> do the same for the API?
>

Unlike code, the problem is less about it being "sloppy" and more
about it being "imperfect". A lot of less-than-perfect code is
accepted because it meets all requirements, and that's fine. I'd like
to aim higher with APIs, where we try to meet all requirements and
also excel at them. That's easiest to do with a concerted
collaborative effort which has become... difficult. With
QFileSelector, I had a hard time nailing down the interested reviewers
and had no opportunity, as I had in Nokia, to trap them in a meeting
room until I saw the understanding exuding from their eyeballs.
Without an in-depth discussion, it's hard to tell if I hit upon the
perfect API (unlikely) or what the weaknesses are in the current
proposed API which keeps people from getting interested. Mailing list
hasn't worked too well, if only because of my inherent tendency to
write long paragraphs. We don't want my Qt APIs to suffer just because
I moonlight as a novelist ;) .

Just brainstorming; here are some ideas on ways we could improve API reviews:

- Have an "API review board", and for substantial new APIs (e.g., a
new class) everyone from the review board should comment on the gerrit
patch (even if it's just "+1"). This wouldn't help in finding the
interested parties, but a well rounded review board would at least
look at the API from several angles. For example, I could be on that
list to review new APIs from a QML perspective.

- Require a combined score of +6 for new APIs on gerrit. That ensures
that at around 3 people have reviewed it,  (either 3 reviewing
thoroughly, or one thoroughly and 5 partially). Could get confusing
because it would be the first time that the numbers interact
arithmetically.

- Have IRC meetings (or audio/video conferences) after each minor
version alpha release, giving a further review to all new APIs in that
release and honing it for beta. While the real time chat can be nice,
it does automatically preclude certain timezones so keep that drawback
in mind.

- Amend the commit policy to require real-world examples (not
necessarily as new examples in the qt repo) for all new APIs, so that
no API is added without at least one user-visible usecase being fully
worked through. Depending on how far you take "real world", this could
be a lot of work...

Any of these sound good to anyone?

Does anyone have another idea on how to improve API reviews in a
distributed manner?

--
Alan Alpert