[Development] (To what extent) Should we start the API change review earlier ?

[Volker Hilsheimer]

> On 2 May 2023, at 11:36, Edward Welbourne <edward.welbourne at qt.io> wrote:
>> This email is the anchor for various topics. I’ll start with a few
>> threads with things that I remember and feel could be improved. If you
>> have something new, please start a new thread in reply to this email, ...
> 
> OK, then (see Subject):
> 
> We could simply start the review earlier.  The only real significance of
> the feature-freeze is that it's the cut-off for changes to API, aside
> from the changes to changed API that get prompted by the review.  So we
> could perfectly well start that earlier, and maybe get some of the fixed
> changes in before feature-freeze.  OTOH, that would be more work for the
> release team, having to update the reviews more times.
> 
> Another approach would be for Maintainers to run the scripts [0] on
> their modules locally and have a look for anything in the result that
> looks like it needs more eye-balls on it.  They can either dig up the
> original gerrit reviews of the changes and drag relevant eye-balls to
> take a look at that, or fake up a commit based on the one the
> api-review-gen script produced, that captures the particular piece(s)
> they want reviewed and push that to gerrit.
> 
> [0] they're in qtqa/scripts/api-review/, stick that in your PATH and run
>    api-review-gen --help
>    for details.  You'll need the dulwich python package installed.
> 
> Incidentally, if anyone does run these scripts for themselves and finds
> difficulties with that, I'd love to hear from you, via Jira if you think
> it's worth it.  The "break up git modules by Qt module" feature request
> already exists, QTQAINFRA-4763.
> 
> What (else) could we do to make the review run more smoothly and quickly ?


The primary purpose of the header review is to catch *technical* mistakes - BC or SC breakages - rather than to discuss API design, naming, or style.

So starting the header review process when we still expect changes might not be useful. I think starting it when we are in beta makes sense.

But the reality is that the header review got rather conflated with API review (and, as far as template code in headers is concerned, even implementation review) in the last iterations. And while that is sometimes ok, the header review isn’t the right process to discuss the design of more complex frameworks.

Instead of relying on the header review for that, it would be great if we could have more API reviews. For anything non-trivial it helps a lot to have the maintainer of the module and/or author of the new stuff walk through the changes. The diff that the tools for the header review generate might help facilitating that of course. IRC as the lowest common denominator is … not great for that; within TQtC we have MS Teams which the build system team used a while ago to discuss the new cmake APIs, also inviting contributors outside.

Was that acceptable and useful? Can we do that more? For people outside TQtC that have some new API to discuss, someone inside can always help with organising, so that we don’t get blocked by some paralysing discussion about what platform we should use for this.


Volker