[Development] HEADS-UP: Qt 6.8 API change review

[Edward Welbourne]

Volker Hilsheimer (10 June 2024 10:38) wrote:
> We added QT_TECH_PREVIEW_API to 6.7 after 6.7.0 (because the tag
> wasn’t available in time for 6.7.0) so that we can see that difference
> when we remove it against from 6.8 (with the intention of informing
> reviewers about that change), and that’s now completely invisible.

That's a case for special treatment for the present release; at which
patch release did this new tag get added ?  Do we intend that normally
the M.m.0 release shall contain this tag for tech preview additions, or
are they apt to be added for later M.m.p releases ?

> Also header changes in 6.7 might be fixes (such as
> https://codereview.qt-project.org/c/qt/qtbase/+/565633, which
> shouldn’t have shown as a diff here).

It's a change since 6.7 had its API change review, that I suppose has
previously only been reviewed in the normal course of code review.  The
fact that it has also landed in 6.7 (because it's a fix) doesn't mean
it's been part of a pre-release API change review.

> In short, I think we should review the delta between current 6.8 and
> current 6.7 (or at most latest 6.7 patch release), not 6.7.0.

This means that, if someone has made an ill-advised change to public API
and picked it to the stable branch, no pre-release change review shall
see it.  That is a significant material change to process.

> If we think that we should formally review changes within 6.7 patch
> cycles (ref discussion about permitting forward BiC breakage), then we
> should not abuse the 6.8 header review, IMHO. The change analyser tags
> changes in headers that get cherry-pick’ed with "API change
> cherry-picked”, so
> https://codereview.qt-project.org/q/hashtag:%22api+change+cherry-picked%22+status:merged
> lists all those.

I think, once we've been doing this through several minor versions, that
filter is going to produce a lot of matches: and it'll be very
non-obvious (in that view) which release each entry in it landed in, so
I am sceptical about it being helpful in practice.  It also requires a
reviewer to wade through the whole list, one review at a time, rather
than gathering these changes all in one place.

One could, after all, say that
https://codereview.qt-project.org/q/hashtag:%22needs+api-review_6.8%22+status:merged
makes the 6.8 API change review redundant.  While I'm sure Jani and I
would be very happy to dispense with the whole API change review
process, as a whole lot of work and stress, I am not convinced that
argument really works.

So yes, reviewing API changes relative to the last result of an API
change review is going to mean re-reviewing things folk have reviewed
before, but by gathering all of them in one place the process ensures
those who take part in these reviews do see all of the changes between
major releases.  I am wary of the suggestion that we can weaken that
requirement.

We could, of course, potentially make the API change review produce (up
to) two changes per Qt module, instead of one: the first covers 6.7.0
through 6.7.latest, the second 6.7.latest through 6.8.  That's probably
something I could teach the scripts to do, and it'd show your additions
of tech-preview tags in the first and their removal in the second, for
example.  This would ensure API change reviewers do see all changes and,
by splitting them up, with the first hopefully trivial, it'd reduce the
second to a more manageable level.

	Eddy.