Re: [Development] Behavior-changing bugfixes in patch-level releases
> -Original Message-
> From: Arno Rehn
> Sent: Wednesday, July 12, 2023 4:13 PM
> To: EXT Mitch Curtis ; Qt development mailing list
>
> Subject: Re: [Development] Behavior-changing bugfixes in patch-level releases
>
> Hi Mitch,
>
> On 12.07.2023 09:52, EXT Mitch Curtis wrote:
> >> Context: We have been hit by
> >> https://codereview.qt-project.org/c/qt/qtdeclarative/+/472596
> >> (which is even marked as "Important Behavior Change") ending up only
> >> in 6.5.1. It was quite the headache figuring out that 6.5.0 ->
> >> 6.5.1 has broken part of our ListViews.
> >
> > I'm sorry to hear that.
> >
> > As the developer who approved the change, I've left my thoughts
> > here:
> >
> > https://bugreports.qt.io/browse/QTBUG-
> 114166?focusedId=734562=com
> > .atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-
> > 734562
> >
> > In summary, I believe the official branch policy was followed here.
> >
> > I'm not sure if your use case is the same as the snippet in the bug
> > report, so it would be useful if you could share it so that we can see
> > if there's a legitimate usage that we've broken. That's not to say
> > that it's good to introduce regressions, of course, but often it's
> > hard to know how users are using APIs and we often only find out after
> > the change has been merged, because auto tests can't cover every use
> > case for the same reason.
>
> We have a vertical ListView which shows icon-only buttons. The buttons all
> have the same width, but depending on the icon size and/or the style, that
> width may differ. So we update the contentWidth of the ListView as soon as
> the first button is loaded. Something like this:
>
>
> ListView {
> id: sidebar
> Layout.fillHeight: true
> implicitWidth: contentWidth
>
> model: // ...
>
> delegate: IconButton {
> icon.name: // ...
> onImplicitWidthChanged: if (sidebar.contentWidth < 0) {
> sidebar.contentWidth = implicitWidth
> }
> }
> }
>
> Maybe we're doing this wrong; but I didn't find any other viable way to have a
> ListView auto-determine the size in the non-scrollable dimension based on its
> content.
I'd recommend setting a size on the ListView that doesn't depend on the
implicit size of the delegates; one that is calculated with TextMetrics, for
example. The reason for this is that it's not always possible to know the size
of every delegate at startup. While scrolling a view, delegates that are larger
or smaller might be loaded.
To use a more recent example where this problem had to be solved,
https://doc.qt.io/qt-6/qml-qtquick-tableview.html#row-heights-and-column-widths
says:
"When a new column is flicked into view, TableView will determine its width
by calling the columnWidthProvider. If set, this function will alone decide the
width of the column. Otherwise, it will check if an explicit width has been set
with setColumnWidth(). If not, implicitColumnWidth() will be used. The implicit
width of a column is the same as the largest implicit width found among the
currently loaded delegate items in that column. Trying to set an explicit width
directly on a delegate has no effect, and will be ignored and overwritten. The
same logic also applies to row heights."
Note the part that says:
"The implicit width of a column is the same as the largest implicit width
found among the currently loaded delegate items in that column."
There's also a note below:
"Note: The resolved width of a column is discarded when the whole column is
flicked out of the view, and is recalculated again if it's flicked back in.
This means that if the width depends on the implicitColumnWidth(), the
calculation can be different each time, depending on which row you're at when
the column enters (since implicitColumnWidth() only considers the delegate
items that are currently loaded). To avoid this, you should use a
columnWidthProvider, or ensure that all the delegate items in the same column
have the same implicitWidth."
This is also described in e.g. https://bugreports.qt.io/browse/QTBUG-76830, and
documented in
https://doc-snapshots.qt.io/qt6-dev/qml-qtquick-listview.html#variable-delegate-size-and-section-labels.
The cacheBuffer workaround mentioned there may help but is not a complete
solution.
From testing locally with a runnable adaptation of your example, I get a
ListView whose width is 12:
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
ApplicationWindow {
visible: true
width: 800
height: 600
title: "width: " + sidebar.width
ColumnLayout {
width: 200
height: parent.height
Label {
text: "Some extra stuff here"
}
ListView {
id: sidebar
implicitWidth: contentWidth
//clip: true
[Development] Relocated QUIPs (was Re: Behavior-changing bugfixes in patch-level releases)
Volker Hilsheimer (12 July 2023 12:21) wrote (inter alia):
> The branch policy lives on
> http://quips-qt-io.herokuapp.com/quip-0016-branch-policy.html
which reminds me: that server is no longer maintained and shall sooner
or later be killed. The current preferred URLs for QUIPs are on
qt-project.org; the one above becomes
https://contribute.qt-project.org/quips/16
and, if you're linking to one from our Wiki, the preferred form is to
use the QUIP template, in any of the following forms:
{{QUIP|16}} -- plain and simple "QUIP 16"
{{QUIP|16|Workflow}} -- link to section (2nd arg is fragment identifier)
"Workflow"
{{QUIP|16||Branch Policy}} -- 3rd arg is link text "Branch Policy"
{{QUIP|16|Workflow|QUIP 16, Branch Policy: Workflow}} -- link to section, give
full title.
Note: don't try to use a | in the link text, as Wiki-syntax will
interpret that as the separator starting yet another parameter to the
template.
Eddy.
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Behavior-changing bugfixes in patch-level releases
Hey, > We do generate release notes for patch release, i.e. > https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/6.5.1/release-note.md > where this particular change is included in the “Important Changes” > section: > https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/6.5.1/release-note.md#qtdeclarative > > The intention of those notes is to make it easier for people upgrading > to evaluate the impact, and to decide whether some extra testing needs > to be done before rolling the new version out to everyone. > > But maybe this is not the most effective way to communicate changes > like this. Are these notes easy enough to find? Is a single “Important > Changes” section sufficient, or do we need more differentiation? Some > important changes are marked as such because they fix a bad bug, > others might be marked as important because they might require > adaptation in existing code that relies on previous behavior. They are > all marked as "Important Behavior Change” in the commit log, and > usually with a module or even type name (in this case, > "[ChangeLog][QtQuick][ListView][Important Behavior Change]”). Our > commit template suggests “[ChangeLog][module][class/topic]”. > > Perhaps the release-notes generation script could group things a bit > better, at least by module rather than by repository, and also > including the class/topic. > > Ideas on how we can make this better are very welcome. It's a tiny change, but personally I'd appreciate if the titles of the "Important changes" entries were formatted in bold. It'd make things a bit easier to scan visually, as that's all you need to get a first impression of "does this affect me in any way?". As for discoverability, I see it's linked in the release blog post. Perhaps it could also be listed in the release announce mails for people who don't click through to the blog post? Finally (not sure if that's already done), for people upgrading using the installer, maybe the installer could link to the release notes or even show them inline? But in the end, I suppose many people (don't necessarily mean the OP here, just in general) just won't read them. Not much you can do about that, I think. Florian pgphithimlpzS.pgp Description: PGP signature -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Behavior-changing bugfixes in patch-level releases
> On 12 Jul 2023, at 09:52, EXT Mitch Curtis via Development > wrote: > > Hi Arno, >> >> Hey everyone, >> >> what is the policy for adding behavior-changing bugfixes to patch-level >> releases? Is this something to expect? >> At the moment we operate under the assumption that bumping the patch- >> level of Qt is pretty much a no-brainer and can be done without having to do >> much in the way of validation. >> >> If behavior changes in patch-level releases are to be expected, we'll have >> to be >> more careful with bumping Qt. >> > > I'm sorry to hear that. > > As the developer who approved the change, I've left my thoughts here: > > https://bugreports.qt.io/browse/QTBUG-114166?focusedId=734562=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-734562 > > In summary, I believe the official branch policy was followed here. Without going into the details of this particular change: The branch policy lives on http://quips-qt-io.herokuapp.com/quip-0016-branch-policy.html and deliberately leaves room for interpretation and case-by-case decision making. From a certain point of view, any bug fix can be seen as a behavior change, and side effects are not unusual, and not always expected or wanted. If a patch comes with a ChangeLog entry, then bringing it back into an “LTS Strict” (or even “Very Strict”) branch should not be done lightly. But in general, we do want to be able to make such fixes also in older branches. We do generate release notes for patch release, i.e. https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/6.5.1/release-note.md where this particular change is included in the “Important Changes” section: https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/6.5.1/release-note.md#qtdeclarative The intention of those notes is to make it easier for people upgrading to evaluate the impact, and to decide whether some extra testing needs to be done before rolling the new version out to everyone. But maybe this is not the most effective way to communicate changes like this. Are these notes easy enough to find? Is a single “Important Changes” section sufficient, or do we need more differentiation? Some important changes are marked as such because they fix a bad bug, others might be marked as important because they might require adaptation in existing code that relies on previous behavior. They are all marked as "Important Behavior Change” in the commit log, and usually with a module or even type name (in this case, "[ChangeLog][QtQuick][ListView][Important Behavior Change]”). Our commit template suggests “[ChangeLog][module][class/topic]”. Perhaps the release-notes generation script could group things a bit better, at least by module rather than by repository, and also including the class/topic. Ideas on how we can make this better are very welcome. Volker -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Module maintainers: QT_NO_CONTEXTLESS_CONNECT in your modules
> Am 10.07.2023 um 18:02 schrieb Giuseppe D'Angelo via Development: >> Hi, >> >> https://codereview.qt-project.org/c/qt/qtbase/+/487560 introduces >> QT_NO_CONTEXTLESS_CONNECT , a macro that disables the 3-arguments connect -- >> in other words, it disables the >> >>> QObject::connect(sender, signal, functor) >> >> overload, leaving only 4/5 argument(s) overloads >> >>> QObject::connect(sender, signal, receiver/context, functor/slot, (type)) >> >> >> as well as the string-based connect(). >> >> >> The reason for NOT using the 3-args overload is that it is error prone. >> >> For starters, it makes it hard to reason about the lifetime of such a >> connection. It makes it very easy to connect to lambdas that capture some >> local state in the receiver, but when the receiver is destroyed, the >> connection isn't automatically disconnected (therefore, if the signal is >> emitted, the program will crash). Fixing this may or may not be >> straightforward, depending on how much state is captured. >> >> Second, it's also easy to forget that since there's no receiver/context, the >> connection is always forced to be *direct*, which complicates things if >> multiple threads are involved. >> >> -- >> >> I'm about to enable the macro for (most) of QtBase: >> >> https://codereview.qt-project.org/c/qt/qtbase/+/489232 >> >> I've also done the exercise of enabling it for qtdeclarative (patches >> pending) and qttools (merged), but there's simply too much code out there >> that *still* uses it. As I said above, fixes are not always obvious >> (sometimes it's very unclear who the "receiver" is). >> >> -- >> >> Since this is ultimately a code style issue, I'd like to leave the decision >> with the module maintainers regarding whether or not enable >> QT_NO_CONTEXTLESS_CONNECT for their own modules. Ideally, however, we should >> enable it in headersclean, so please make at least those not contain 3-args >> connect(). >> > On 11 Jul 2023, at 11:56, Hasselmann Mathias via Development > wrote: > > Hi Peppe, > > thank you alot for this highly appreciated and super useful feature! > Give up counting how often I forgot the context argument. > Thanks Peppe for taking the initiative! The change has now merged for qtbase, and given the war stories shared here it would indeed be great if we can all clean up the remaining modules so that we can enable this as widely as possible. Volker -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Behavior-changing bugfixes in patch-level releases
Hi Mitch,
On 12.07.2023 09:52, EXT Mitch Curtis wrote:
Context: We have been hit by
https://codereview.qt-project.org/c/qt/qtdeclarative/+/472596
(which is even marked as "Important Behavior Change") ending up
only in 6.5.1. It was quite the headache figuring out that 6.5.0 ->
6.5.1 has broken part of our ListViews.
I'm sorry to hear that.
As the developer who approved the change, I've left my thoughts
here:
https://bugreports.qt.io/browse/QTBUG-114166?focusedId=734562=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-734562
In summary, I believe the official branch policy was followed here.
I'm not sure if your use case is the same as the snippet in the bug
report, so it would be useful if you could share it so that we can
see if there's a legitimate usage that we've broken. That's not to
say that it's good to introduce regressions, of course, but often
it's hard to know how users are using APIs and we often only find out
after the change has been merged, because auto tests can't cover
every use case for the same reason.
We have a vertical ListView which shows icon-only buttons. The buttons
all have the same width, but depending on the icon size and/or the
style, that width may differ. So we update the contentWidth of the
ListView as soon as the first button is loaded. Something like this:
ListView {
id: sidebar
Layout.fillHeight: true
implicitWidth: contentWidth
model: // ...
delegate: IconButton {
icon.name: // ...
onImplicitWidthChanged: if (sidebar.contentWidth < 0) {
sidebar.contentWidth = implicitWidth
}
}
}
Maybe we're doing this wrong; but I didn't find any other viable way to
have a ListView auto-determine the size in the non-scrollable dimension
based on its content.
--
Arno Rehn
Tel +49 89 189 166 0
Fax +49 89 189 166 111
a.r...@menlosystems.com
www.menlosystems.com
Menlo Systems GmbH
Bunsenstrasse 5, D-82152 Martinsried, Germany
Amtsgericht München HRB 138145
Geschäftsführung: Dr. Michael Mei, Dr. Ronald Holzwarth
USt.-IdNr. DE217772017, St.-Nr. 14316170324
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Behavior-changing bugfixes in patch-level releases
Hi Arno, > -Original Message- > From: Development On Behalf Of > Arno Rehn > Sent: Wednesday, July 12, 2023 3:01 PM > To: Qt development mailing list > Subject: [Development] Behavior-changing bugfixes in patch-level releases > > Hey everyone, > > what is the policy for adding behavior-changing bugfixes to patch-level > releases? Is this something to expect? > At the moment we operate under the assumption that bumping the patch- > level of Qt is pretty much a no-brainer and can be done without having to do > much in the way of validation. > > If behavior changes in patch-level releases are to be expected, we'll have to > be > more careful with bumping Qt. > > Context: We have been hit by > https://codereview.qt-project.org/c/qt/qtdeclarative/+/472596 (which is > even marked as "Important Behavior Change") ending up only in 6.5.1. > It was quite the headache figuring out that 6.5.0 -> 6.5.1 has broken part of > our ListViews. I'm sorry to hear that. As the developer who approved the change, I've left my thoughts here: https://bugreports.qt.io/browse/QTBUG-114166?focusedId=734562=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-734562 In summary, I believe the official branch policy was followed here. I'm not sure if your use case is the same as the snippet in the bug report, so it would be useful if you could share it so that we can see if there's a legitimate usage that we've broken. That's not to say that it's good to introduce regressions, of course, but often it's hard to know how users are using APIs and we often only find out after the change has been merged, because auto tests can't cover every use case for the same reason. > Regards, > Arno > > -- > Arno Rehn > Tel +49 89 189 166 0 > Fax +49 89 189 166 111 > a.r...@menlosystems.com > www.menlosystems.com > > Menlo Systems GmbH > Bunsenstrasse 5, D-82152 Martinsried, Germany Amtsgericht München HRB > 138145 > Geschäftsführung: Dr. Michael Mei, Dr. Ronald Holzwarth USt.-IdNr. > DE217772017, St.-Nr. 14316170324 > -- > Development mailing list > Development@qt-project.org > https://lists.qt-project.org/listinfo/development -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
[Development] Behavior-changing bugfixes in patch-level releases
Hey everyone, what is the policy for adding behavior-changing bugfixes to patch-level releases? Is this something to expect? At the moment we operate under the assumption that bumping the patch-level of Qt is pretty much a no-brainer and can be done without having to do much in the way of validation. If behavior changes in patch-level releases are to be expected, we'll have to be more careful with bumping Qt. Context: We have been hit by https://codereview.qt-project.org/c/qt/qtdeclarative/+/472596 (which is even marked as "Important Behavior Change") ending up only in 6.5.1. It was quite the headache figuring out that 6.5.0 -> 6.5.1 has broken part of our ListViews. Regards, Arno -- Arno Rehn Tel +49 89 189 166 0 Fax +49 89 189 166 111 a.r...@menlosystems.com www.menlosystems.com Menlo Systems GmbH Bunsenstrasse 5, D-82152 Martinsried, Germany Amtsgericht München HRB 138145 Geschäftsführung: Dr. Michael Mei, Dr. Ronald Holzwarth USt.-IdNr. DE217772017, St.-Nr. 14316170324 -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
