[Development] QML API Review

[Shawn Rutledge]

> On 31 Aug 2018, at 13:39, Kai Koehne <Kai.Koehne at qt.io> wrote:
> 
> Hi,
> 
> In addition to the C++ API review that Edward has been pushing, we also need to review new/changed QML API.
> 
> Most of our QML API is actually defined in C++, so one way to review them is diffing the corresponding .h files; however, because they are private (in the C++ sense), every module is free to place these files in different locations. It also misses the information on how exactly the type is registered in the end (think of qmlRegisterXXX).
> 
> Fortunately, we have another way to inspect QML API: The plugins.qmltypes files that every plugin is supposed to ship alongside. The prime use of these files is to make syntax highlighting and code completion in Qt Creator possible. However, it turns out it's also a very concise way of reviewing new API, so let's use it for that 😊.

Yep we got started with that already (at least I found a few things I want to change).

> The .qmltypes files need to be updated manually. I've now run the update for all the .qmltypes files I could find and upload the change for review (see also https://bugreports.qt.io/browse/QTBUG-70264 and linked patches). I suggest to use these patches as the basis for reviewing the QML API. Different from C++, I suggest that if you're happy with the API, please +2 it - The patch will actually be merged in the end.

But after the changes do you want to rebase and update that same patch, or just have another patch?  I was thinking that after API review, changes are likely.  So if you want to combine all the changes, your patch will end up dangling for a few weeks or months.  I was thinking we could do it twice, once before API review and once before release.  On the other hand, if we keep it atomic, then running “gitk plugins.qmltypes” will make it easy to re-review in the future what exactly changed between releases.  Maybe that’s a nice feature to have, if it’s worth the trouble?

> Note that some the plugins.qmltypes files got updated in between, and therefore the patches do not show the full diff to version 5.11. Other plugins.qmltypes files didn't see an update in 5.11, something we should really avoid in the future ☹ In these cases a separate review might be necessary.
> 
> Anyhow, it turns out that most developers don't update the plugins.qmltypes files, so I suggest to establish this as the new norm, and update them only before the release, as part of the API review process.
> 
> An alternative would be to make sure that the .qmltypes files are up to date directly before the API review, and include them as part of the C++ diff. Anyhow, this would require us to _not_ instantly fix things in the .qmltypes file when it gets updated, but first check in the plugins.qmltypes file as is - something that can be difficult to accept. So maybe it's just easier to have this as separate diffs.

You could let git diff do the work: it can compare the latest qmltypes files against the latest on 5.11 branch regardless how many patches modified them.  And that could be added to the tool that generates the header diffs.