[Development] Qt 6.1.0 release note

[Giuseppe D'Angelo]

Hello,

Il 28/04/21 11:11, Jani Heikkinen ha scritto:
> Hi!
> 
> And thanks for your feedback! Some comments below.
> 
> 
>> -----Original Message-----
>> From: Development <development-bounces at qt-project.org> On Behalf Of
>> Giuseppe D'Angelo via Development
>> Sent: tiistai 27. huhtikuuta 2021 17.54
>> To: development at qt-project.org
>> Subject: Re: [Development] Qt 6.1.0 release note
>>
>> Hi,
>>
>> On 27/04/2021 11:53, Jani Heikkinen wrote:
>>>
>>> Initial release note for Qt 6.1.0
>>> here:https://code.qt.io/cgit/qt/qtreleasenotes.git/tree/qt/6.1.0/relea
>>> se-note.txt
>>
>> I must say, I don't quite like the format used here.
>>
>> While having the commit SHA might be useful if someone wants to look up
>> the commit corresponding to each changelog entry, there's a few things that
>> don't quite sound OK to me.
>>
>> 0) No 72 columns wrapping. There, I said it.
> 
> That's true. We can easily add this but is this really needed nowdays anymore (with big 4k screens)?

It is. That's why web browsers, word processors etc. feature a "reading 
mode" where they limit the horizontal text width.

> https://blog.mozilla.org/firefox/reader-view/



And that's why most websites don't have content in full width, including 
Qt's own docs:

> https://doc.qt.io/qt-5/qprinter.html

But also documentation at large, and newspapers, etc.

> http://eel.is/c++draft/concepts

> https://docs.python.org/3/extending/index.html

> http://docs.gl/gl4/glActiveTexture

> https://www.theguardian.com/environment/shortcuts/2017/jan/08/colour-changing-cats-warn-radioactive-waste-nuclear-plants-distant-descendants


>>
>> 1) Quoting the commit message of a given change is hardly useful. That's why
>> we mandate a [ChangeLog] entry, after all; commit messages must fit in 72
>> columns, so they'll use a bunch of shortcuts that shouldn't be read as-is by
>> end users:
>>> 427da06414 QRE: discourage users from assuming that QRE stores the
>>> subject > 5a0e5521e4 QVectorND: make some constructors explicit
>>> 86729df9d4 QTextHtmlParserNode: Limit colspan to avoid segfault
>>
>> Best case scenario, they just repeat what's much better explained by the
>> ChangeLog message. So why adding the commit message at all?
> 
> Well, there is other opinions as well. With commit sha and commit message reader will be easy to check the change & get more information if needed. Ok, maybe commit message doesn't help that much but on the other hand it doesn't harm either...

It seems to be completely redundant information, and also lacks decent 
formatting.

As I said, it's written to be read by a developer, so it takes 
shortcuts. "QFoo: fix segfault" is acceptable in a commit's title. A 
changelog in that commit could contain a more elaborated "A crash has 
been fixed when passing non-convex polyhedra to QFoo. Note that this 
usage is still discouraged, as QFoo would exhibit quadratic behavior.".

If there's changelog text, then no need to use the commit's title in the 
release files.


>> 3) The messages are grouped by submodule. But submodules are not
>> relevant for end-users, end of story. Why there isn't a grouping per Qt
>> module instead? We force people to add that to ChangeLogs, only to throw
>> away that information?
>>
>> 3a) What about "meta" changes, like the ones we list under [Important
>> Behavior Changes], or [Third-Party code] or whatever? Now they're thrown
>> into the mix, so they don't stand out as things to watch for. I can hardly
>> imagine that I need to dig into the middle of [qtdeclarative] only to find a
>> behavioral change in the QML language.
> 
> It depends. Grouping based submodule makes it clear for the user in which repo the change is. User can't checkout the qt module but the submodule and there changes are. But I agree it could be useful to parse also tags after that [ChangeLog] tag & group entries based on that under the submodule. But to be honest those [ChangeLog] entries varies so much that it is really hard to produce clear grouping & information based on that; sometimes there is only [ChangeLog] tag, next tag after it isn't a qt module but something else etc. So producing a note which doesn't necessarily need an manual formatting isn't that straightforward (we have seen that in the past with those old changes files).

The repository is not relevant information for the user. Users don't use 
repositories (and shouldn't care about repositories), they use module 
names.

If the format of ChangeLog entries in commit messages is inconsistent to 
the point that it requires lots of manual adjustments, isn't it 
something that the sanity bot could also help check?



>> 4) Also why not grouping by class? If now I want to find all the changes to
>> QString, I have to read the entire [qtbase] thing. Again this is information
>> present in the [ChangeLog]s that is getting binned.
> 
> A comment above pretty much answers to this already. And in addition: Most probably there isn't [ChangeLog] tags in every QString related changes so you can't get all those anywhere but from git log

That's not the point; the point is that the changes to the same class 
are not visually grouped together. (I didn't check if they're actually 
grouped in the current format, as it's not indicated anywhere). If there 
are N changes to QFoo, why are they scattered over QFoo's submodule, 
rather than being grouped together in a subsection called "QFoo"?



>>
>>> Target is to release Qt 6.1.0 Thu 6th May and so on it would be good to
>> have needed updates in before that.
>>
>> So not even a RC2 after, indeed, changing the behavior of the top X most
>> used classes in Qt? (Certainly of the most used one.) Oh well, I won't
>> repeat my (unanswered) questions and remarks from QTBUG-91360.
> 
> RC2 will be published later this week with the fixes for this one.

OK, thank you; from the message it looked like that there was just the 
final release out.

> 
> So let's improve the release note to get it as good as it can be. The idea is that we can even update the already released release's note if really needed. So we don't need to postpone the releasing even if the note isn't as good as it can be. And at least coming releases will have the better one. Just let's agree what needs to be done and we try to implement improvements after that. And meanwhile anyone can manually add updates to existing one(s) for review.
> 
> I attached a sample of parsed [ChangeLog] entries from qtbase to indicate the inconsistency of entries. How the script should parse lines e.g. 7, 11, 21, and 28 ?

It seems to me that they're all violating the commit policy, 
specifically the ChangeLog policy, one way or another:

https://quips-qt-io.herokuapp.com/quip-0017-Change-log-creation.html

And they look all checkable by a bot.


Line 7:
> ['[ChangeLog][QCosmeticStroker] Pen patterns are restrained to a\nmaximum length and values of 1024, fixing oss-fuzz issue 25310.\n\n']

missing the module indication (QtGui).


Line 11:

> ['[ChangeLog][QMetaProperty][Important Behavior Change]\nQMetaProperty::typeName returns now always the same name as name() of the\ncorresponding metatype. This can cause a change for enum properties\nwhich were not fully-qualified.\n\n']

Mix/match of tags. QUIP17 doesn't clarify this, but it should, i.e. 
should IBC be the top-level tag, followed by the class tag? Or no class 
tag after IBC? It ultimately depends on the format we want to give to 
the output.

(Also, 'Change' has been misspelled -- it's 'Changes'.)


Line 21:

> ['[ChangeLog][Third-Party Code] PCRE2 has been updated to version\n10.36.\n\n']

Looks legit to me.


Line 28:

> ["[ChangeLog][Potentially Source Breaking Change] It was possible to\ncreate a QPropertyBinding from a property; this would steal any set\nbinding from the property or create an invalid binding if none was set.\nUse makePropertyBinding if you want to to create a binding which depends\non the property's value, or takeBinding if you want to repurpose the\nproperty's binding.\n\n"]

Also in principle legitimate, but again misspelled: the correct one 
would be 'Potentially Source-Incompatible Changes'.


In short: if the ChangeLogs in the commit messages are so "bad" that 
they make the generation of the release files a burden, then we have to 
fix the format and/or the process (given all these belong to +2'd 
commits); and not make the release files worse.


My 2 c,

-- 
Giuseppe D'Angelo | giuseppe.dangelo at kdab.com | Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4329 bytes
Desc: Firma crittografica S/MIME
URL: <http://lists.qt-project.org/pipermail/development/attachments/20210428/e58d5764/attachment.bin>