Hello,

Il 28/04/21 11:11, Jani Heikkinen ha scritto:
Hi!

And thanks for your feedback! Some comments below.


-----Original Message-----
From: Development <[email protected]> On Behalf Of
Giuseppe D'Angelo via Development
Sent: tiistai 27. huhtikuuta 2021 17.54
To: [email protected]
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 | [email protected] | 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

Attachment: smime.p7s
Description: Firma crittografica S/MIME

_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development

Reply via email to