Re: [Development] (To what extent) Should we start the API change review earlier ?
Volker Hilsheimer (9 May 2023 17:01) wrote: > 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. Some technical errors are future-readiness: as long as we have BC and SC commitments, we have to think about whether what we design today will leave us with a BC or SC constraint preventing us from improving on today's design later, when new requirements surface or we discover any limitations of the initial design. Consequently, discussions of API design, naming and even style can be relevant to determining whether we have made a technical error in the present design. > 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. We certainly can't close a review until after FF, even if we do start one before. The advantage of starting early is to give more time for the conversations to happen and the consequent fixes to be sorted out. > 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. Obviously the best time to review a new API or a change to API is when it is first introduced, long before FF or the API change review. All the same, the latter can bring more eyes to the change and more insights into potential problems with the change. The initial change was typically reviewed by folk working closely with the particular part of the code-base; the API change review is apt to bring the change to the attention of potential clients of the new API, or to folk who've previously designed similar-shaped APIs for other purposes, who may have valuable insights into the new design. OTOH, when we're all rushing headlong towards the deadline of FF, an early start to the API change review might be of limited value, simply because relevant folk are too busy to have those conversations. And the part of the process which has been slow at recent releases has been getting all the modules signed off by their respective Maintainers; it's not really clear to me that an earlier start would help with that. Eddy. -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] (To what extent) Should we start the API change review earlier ?
> On 2 May 2023, at 11:36, Edward Welbourne 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 -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
[Development] (To what extent) Should we start the API change review earlier ?
Volker Hilsheimer (2 May 2023 10:57) wrote: > With Qt 6.5 out for a while already, and roughly a month to go until > Qt 6.6 feature freeze and the start of the various activities that > lead up to the release, it’s perhaps not too early to review some of > the pain points we experienced with 6.5, and discuss how we can > improve. One pain point that came up at the time was that the reviews don't get enough attention; another that we maybe need more time. I think the first of those is somewhat covered by one of your other sub-threads. > In particular, the header review was particularly laborious this time > around, and I would like to see what we can do to make it less so next > time around. I like that plan. > 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 ? Eddy. -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
