Re: Fixing QNetworkAccessManager use

[Ben Cooksley]

On Thu, Feb 20, 2020 at 9:58 AM Friedrich W. H. Kossebau
 wrote:
>
> Am Mittwoch, 19. Februar 2020, 21:01:20 CET schrieb Johan Ouwerkerk:
> > On Wed, Feb 19, 2020 at 2:09 PM Friedrich W. H. Kossebau
> >
> >  wrote:
> > > Personally I am still unsure what the actual issue is. Why are redirects
> > > needed at all. Why all the address changes all the time?
> >
> > It is part of the HTTP spec for servers to be able to inform clients
> > that resource /foo/bar has moved to /bar/baz, either temporarily or
> > permanently.
>
> :) Thanks for that explanation, but that was not my question here (that part I
> am well aware of, done my share of web stuff).
>
> It was rather: why are subdomain names and/or access paths not once properly
> designed, but instead changed so often that redirection seems so important to
> be a default feature? Just because one can?

Things don't change extremely often.
Sometimes however requirements or other factors change, which
necessitates changing where a resource is hosted.

When this happens, it is extremely useful to have the ability to
relocate it elsewhere.

To use an example, when we first setup files.kde.org it was used by a
couple of things, including Necessitas for the Qt binaries that get
downloaded on to end user (Android) devices. When this was first
established, traffic was well within the reasonable bounds we had
expected when setting this up, and everything was served directly by
our (single) server. This went quite well for a while.

Sometime a bit later, an application was released on Google Play that
used Necessitas which was *extremely* popular, to the extent it caused
around a terabyte of data to be used within 48 hours or so. Hetzner
bandwidth was at this time not only limited to 100mbps, but also
capped - with the limit being 5 TB per month and overage after that
resulting in a charge per terabyte.

We therefore made the decision to convert files.kde.org to a mirror
network (like was already in place for download.kde.org), with
redirection taking place using Mirrorbrain. We were able to complete
this transition quickly thanks to the generous support of some of our
mirrors who established mirrors of files.kde.org. Fortunately
Necessitas had full support for handling redirects, so this is
something we were able to accomplish without any issues.

Had redirect support not been available, we would have been left with
no way out at that time.

I also have other examples involving Marble (including where we got
bitten by QNetworkAccessManager for the very first time - back in
January 2012) and numerous other KDE Edu applications (all of which
fortunately avoided QNAM).

> When we write code, we try to keep API stable as much as possible, and only
> change API when really useful, and that means for the consumer. When doing
> references in text we try to have eternally stable pointers (thanks ISBN &
> Co.),
>
> But this request for stable URLs on the internet might be an idealistic fight
> against windmills of a web 1.0 person...
>
> Cheers
> Friedrich
>
>

Cheers,
Ben

D27152: Introduce FilesystemEntry class

[David Hallas]

hallas added inline comments.

INLINE COMMENTS

> meven wrote in filesystem_entry.cpp:47
> This sound sane to me.
> Ideally we would store only the string in case of an unknown fs or the enum 
> but not both for a same entry, the choice could be done in ctor.

I have added a new enum class type `FilesystemType` to represent the known 
filesystem types. Currently I have just added a single known filesystem type, 
but this should be extended over time. I have also added the necessary from/to 
string conversion functions.

> bruns wrote in filesystem_entry.h:34
> wrap long lines, also below.

I have wrapped all lines to fit 80 characters in width

> bruns wrote in filesystem_entry.h:48
> Whats wrong with mountPoint? Its used everywhere else here, and is a well 
> known term.

Good point :)
Actually, I don't know why I called the function `mountPath` instead of 
`mountPoint` - but I have reamed it, along with the member etc.

> bruns wrote in fstabhandling.cpp:259
> detaches m_fstabCache, also below.

The code didn't need the for loop anyway, so I have rewritten it

> meven wrote in fstabhandling.cpp:388
> Detaches m_mtabCache, just qAsConst or temporary const variable

The code didn't need the for loop anyway, so I have rewritten it

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D27152

To: hallas, #frameworks, bruns, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27504: WIP: RFC: smb faster copy to local

[David Hallas]

hallas added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kio_smb_dir.cpp:49
> Segment does not free its memory. Why not use QByteArray or similar with 
> automatic storage allocation?

Yes, you could (and should ;) ) use standard C++ for this, i.e.:

  std::unique_ptr buf;
  buf = std::make_unique(segmentSize);

> kio_smb_dir.cpp:168
> +std::condition_variable m_cond;
> +Segment *m_buffer[4]; // should be at least 3 to give smbc some read 
> ahead leeway
> +const size_t m_capacity = 4;

Use standard C++ for this, i.e.:

  std::array, m_capacity> m_buffer;

This makes `m_buffer` iterable and it also ensures that the segments are always 
correctly freed.

> kio_smb_dir.cpp:494
> +Buffer buffer(st.st_size);
> +QFuture future = QtConcurrent::run([&buffer, &srcfd]() -> int {
> +while (true) {

What is the reason to use `QFuture` over `std::future`?

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D27504

To: sitter, ngraham, cfeck
Cc: hallas, anthonyfieroni, asturmlechner, kde-frameworks-devel, kfm-devel, 
pberestov, iasensio, fprice, LeGast00n, cblack, MrPepe, fbampaloukas, alexde, 
GB_2, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, 
ngraham, andrebarros, bruns, emmanuelp, mikesomov

D27464: Add application menu dbus paths to org_kde_plasma_window interface

[Carson Black]

cblack marked an inline comment as done.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

[Carson Black]

cblack updated this revision to Diff 76035.
cblack added a comment.


  Use utf8 encoding on client side

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=75855&id=76035

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

[David Edmundson]

davidedmundson added a comment.


  Good start. The direction makes sense.
  
  display.cpp and registry.cpp need updating to the new version number. 
Otherwise we won't be able to use this new method.
  
  It'd be great if you could update the unit test too.

INLINE COMMENTS

> plasmawindowmanagement_interface.cpp:599
> +}
> +org_kde_plasma_window_send_application_menu_changed(resource, 
> qUtf8Printable(service), qUtf8Printable(object));
> +}

We're mixing up our local 8bit and utf8 on different sides of this.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

Re: Fixing QNetworkAccessManager use

[Johan Ouwerkerk]

On Wed, Feb 19, 2020 at 2:09 PM Friedrich W. H. Kossebau
 wrote:
>
> Personally I am still unsure what the actual issue is. Why are redirects
> needed at all. Why all the address changes all the time?
>

It is part of the HTTP spec for servers to be able to inform clients
that resource /foo/bar has moved to /bar/baz, either temporarily or
permanently.
This can be used to do things like mapping /retrieve/document/by/alias
-> /documents/actual/document-id, or to redirect to different hosts
entirely, or to inform plain text HTTP clients to upgrade to using
HTTPS instead. (HSTS is a spec describing how a server can then ask
the client to subsequently enforce its policy preference for when to
connect over HTTPS.)

The main difference between temporary and permanent redirects is that
clients are allowed to "remember" when a resource moved in the case of
permanent redirects so they can optimise subsequent calls to the moved
resources (bypassing the redirect entirely). But as you can see, the
temporary redirect is something that could be used to do load
balancing: assume /resource is expensive to compute or retrieve, then
put a proxy in front which load balances to the actual pool of servers
using temporary redirects. (Of course you could argue that in such a
case maybe round-robin DNS is a better solution altogether.)

Regards,

- Johan

D27514: Fix incorrect use of case statements in accountmodel.cpp

[Bernhard Rosenkraenzer]

bero created this revision.
bero added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bero requested review of this revision.

REVISION SUMMARY
  Building user-manager 5.18.1 with clang++ 10.0 resulted in 2 compiler 
warnings - a quick look at the code shows they're real bugs.
  "case Qt::DisplayRole || AccountModel::FriendlyName:" isn't quite the same as 
"if(role==Qt::DisplayRole || role==AccountModel::FriendlyName)" -- but that's 
clearly what is meant here.

TEST PLAN
  Check the output of AccountModel::data() with various role types, including 
Qt::DisplayRole, AccountModel::FriendlyName, Qt::DecorationRole and 
AccountModel::Face before and after applying the patch.

REPOSITORY
  R128 User Manager

REVISION DETAIL
  https://phabricator.kde.org/D27514

AFFECTED FILES
  src/lib/accountmodel.cpp

To: bero, #frameworks
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27514: Fix incorrect use of case statements in accountmodel.cpp

[Bernhard Rosenkraenzer]

bero edited reviewers, added: Plasma; removed: Frameworks.

REPOSITORY
  R128 User Manager

REVISION DETAIL
  https://phabricator.kde.org/D27514

To: bero, #plasma, #frameworks
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26769: Always strip html if server does not support it

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R289:994f88929a83: Always strip html if server does not 
support it (authored by nicolasfella).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26769?vs=73888&id=76030

REVISION DETAIL
  https://phabricator.kde.org/D26769

AFFECTED FILES
  src/notifybypopup.cpp

To: nicolasfella, #frameworks, broulik, mart
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

Re: Banning QNetworkAccessManager

[Albert Astals Cid]

El dimecres, 19 de febrer de 2020, a les 9:28:22 CET, Volker Krause va escriure:
> Additionally, improved documentation, a possible KNAM and/or driving the QNAM 
> changes upstream can still be done alongside this obviously.

Also for Qt5 which will probably never be changed a clazy warning and making it 
easy to run clazy on gitlab CI would be great.

Cheers,
  Albert

> 
> Regards,
> Volker

Re: Banning QNetworkAccessManager

[Albert Astals Cid]

El dimecres, 19 de febrer de 2020, a les 8:05:01 CET, Ben Cooksley va escriure:
> The easiest path forward is to simply ban QNetworkAccessManager.

Stop saying that, that's not going to happen.

Cheers,
  Albert

> 
> >
> > Regards,
> > Volker
> 
> Regards,
> Ben
> 

Re: Fixing QNetworkAccessManager use

[Friedrich W. H. Kossebau]

Am Mittwoch, 19. Februar 2020, 21:01:20 CET schrieb Johan Ouwerkerk:
> On Wed, Feb 19, 2020 at 2:09 PM Friedrich W. H. Kossebau
> 
>  wrote:
> > Personally I am still unsure what the actual issue is. Why are redirects
> > needed at all. Why all the address changes all the time?
> 
> It is part of the HTTP spec for servers to be able to inform clients
> that resource /foo/bar has moved to /bar/baz, either temporarily or
> permanently.

:) Thanks for that explanation, but that was not my question here (that part I 
am well aware of, done my share of web stuff).

It was rather: why are subdomain names and/or access paths not once properly 
designed, but instead changed so often that redirection seems so important to 
be a default feature? Just because one can?
When we write code, we try to keep API stable as much as possible, and only 
change API when really useful, and that means for the consumer. When doing 
references in text we try to have eternally stable pointers (thanks ISBN & 
Co.),

But this request for stable URLs on the internet might be an idealistic fight 
against windmills of a web 1.0 person...

Cheers
Friedrich

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[Christoph Feck]

cfeck accepted this revision.
cfeck added a comment.


  Merci :)

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  no-css (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27035

To: davidre, #frameworks, ngraham, cfeck
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.13 - Build # 184 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5.13/184/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Wed, 19 Feb 2020 19:29:31 +
 Build duration:
9 min 3 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5TextEditor-5.68.0.xmlcompat_reports/KF5TextEditor_compat_report.htmllogs/KF5TextEditor/5.68.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 62 test(s), Skipped: 0 test(s), Total: 62 test(s)Name: projectroot.autotests.src Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report79%
(22/28)88%
(263/298)88%
(263/298)68%
(33355/49179)50%
(15013/30054)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.src100%
(38/38)100%
(38/38)94%
(4836/5134)49%
(1819/3680)autotests.src.vimode100%
(9/9)100%
(9/9)99%
(5503/5545)58%
(970/1680)src.buffer88%
(15/17)88%
(15/17)90%
(1700/1892)75%
(1103/1472)src.completion100%
(11/11)100%
(11/11)57%
(1785/3131)42%
(1008/2423)src.completion.expandingtree100%
(3/3)100%
(3/3)40%
(182/457)21%
(73/340)src.dialogs0%
(0/4)0%
(0/4)0%
(0/855)0%
(0/180)src.document100%
(4/4)100%
(4/4)61%
(1943/3170)48%
(1431/2965)src.export0%
(0/4)0%
(0/4)0%
(0/121)0%
(0/156)src.include.ktexteditor93%
(14/15)93%
(14/15)91%
(201/220)65%
(145/224)src.inputmode100%
(8/8)100%
(8/8)63%
(192/304)51%
(39/77)src.mode88%
(7/8)88%
(7/8)36%
(378/1050)16%
(146/897)src.part0%
(0/1)0%
(0/1)0%
(0/7)100%
(0/0)src.printing0%
(0/4)0%
(0/4)0%
(0/862)0%
(0/278)src.render100%
(7/7)100%
(7/7)77%
(947/1224)67%
(612/916)src.schema29%
(2/7)29%
(2/7)1%
(19/1468)1%
(6/625)src.script100%
(16/16)100%
(16/16)69%
(716/1038)56%
(226/403)src.search100%
(7/7)100%
 

D27512: README.md: fix example code & code markup

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:8630da5fa573: README.md: fix example code & code 
markup (authored by kossebau).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27512?vs=76019&id=76021

REVISION DETAIL
  https://phabricator.kde.org/D27512

AFFECTED FILES
  README.md

To: kossebau, #kate, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27512: README.md: fix example code & code markup

[Christoph Cullmann]

cullmann accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R39 KTextEditor

BRANCH
  fixreadme

REVISION DETAIL
  https://phabricator.kde.org/D27512

To: kossebau, #kate, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27512: README.md: fix example code & code markup

[Friedrich W. H. Kossebau]

kossebau created this revision.
kossebau added a reviewer: Kate.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  - scope examples with ``` and code type
  - serviceByDesktopName is the correct method
  - drop 0 parameter from createInstance call (nullptr is default arg)
  - drop NO_MODULE from find_package(KF5TextEditor), not really needed, more 
simple example

REPOSITORY
  R39 KTextEditor

BRANCH
  fixreadme

REVISION DETAIL
  https://phabricator.kde.org/D27512

AFFECTED FILES
  README.md

To: kossebau, #kate
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27504: WIP: RFC: smb faster copy to local

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kio_smb_dir.cpp:49
> +: segmentSize(static_cast(segmentSize_)) // won't be <0
> +, buf(static_cast(malloc(segmentSize)))
> +{

Segment does not free its memory. Why not use QByteArray or similar with 
automatic storage allocation?

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D27504

To: sitter, ngraham, cfeck
Cc: anthonyfieroni, asturmlechner, kde-frameworks-devel, kfm-devel, pberestov, 
iasensio, fprice, LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

davidre updated this revision to Diff 76018.
davidre marked 4 inline comments as done.
davidre added a comment.


  Code style and typos

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27035?vs=75717&id=76018

BRANCH
  no-css (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27035

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  src/kmessagewidget.cpp

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D27083: [PC2]Give Label integer width.

[David Edmundson]

davidedmundson added a comment.


  > In my opinion the Plasma Controls need fix
  
  I would approve that.
  
  Though let's be sure to focus on components3.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27083

To: gvgeo, #plasma, #vdg, ndavis, davidedmundson
Cc: ngraham, davidedmundson, kde-frameworks-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
michaelh, ZrenBot, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27083: [PC2]Give Label integer width.

[Nathaniel Graham]

ngraham added a comment.


  Probably `frameworks-plasma | components`

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27083

To: gvgeo, #plasma, #vdg, ndavis, davidedmundson
Cc: ngraham, davidedmundson, kde-frameworks-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
michaelh, ZrenBot, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27083: [PC2]Give Label integer width.

[George Vogiatzis]

gvgeo added a comment.


  In D27083#614285 , @ngraham wrote:
  
  > So the test needs to be fixed?
  
  
  In my opinion the Plasma Controls need fix. They need to always display the 
icons in an real position, no matter what their x position is. Just like qt 
controls.
  Using Layout is a way to workaround the problem.
  
  I closed this patch because it was wrong solution, with the intention to open 
a bug report(with a link to this patch). I have yet to find the proper category.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27083

To: gvgeo, #plasma, #vdg, ndavis, davidedmundson
Cc: ngraham, davidedmundson, kde-frameworks-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
michaelh, ZrenBot, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27508: Remove unnecessary monochrome app icons

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: VDG, ndavis.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  This is a continuation of the ongoing issue we have in reliably making icons 
colorful
  or monochrome in a predictible manner. Currently there are certain app icons 
that have
  monochrome versions, primarily to make them look better in the system tray. 
However other
  monochrome app icons exist for no clear reason. This patch removes those, 
while not
  touching the monochrome app icons.

TEST PLAN
  khelpcenter icon is now consistently colorful in the task manager, window 
decoration,
  and task switcher
  
  One conseauence is that now "Help" menu items and buttons that use the 
`system-help` icon
  are now colorful, which is arguably correct because those actions will open 
the KHelpCenter
  app, and we generally use colorful app icons in places where actions will 
open an app.

REPOSITORY
  R266 Breeze Icons

BRANCH
  remove-unnecessary-monochrome-app-icons (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27508

AFFECTED FILES
  icons-dark/actions/16/help-contents.svg
  icons-dark/actions/22/help-contents.svg
  icons-dark/apps/16/help-browser.svg
  icons-dark/apps/16/hotspot.svg
  icons-dark/apps/16/preferences-system-bluetooth.svg
  icons-dark/apps/16/system-help.svg
  icons-dark/apps/16/virt-manager.svg
  icons-dark/apps/22/help-browser.svg
  icons-dark/apps/22/hotspot.svg
  icons-dark/apps/22/preferences-other.svg
  icons-dark/apps/22/preferences-plugin.svg
  icons-dark/apps/22/system-help.svg
  icons-dark/apps/22/virt-manager.svg
  icons-dark/apps/32/hotspot.svg
  icons/actions/16/help-contents.svg
  icons/actions/22/help-contents.svg
  icons/apps/16/help-browser.svg
  icons/apps/16/hotspot.svg
  icons/apps/16/preferences-system-bluetooth.svg
  icons/apps/16/system-help.svg
  icons/apps/16/virt-manager.svg
  icons/apps/22/help-browser.svg
  icons/apps/22/hotspot.svg
  icons/apps/22/preferences-other.svg
  icons/apps/22/preferences-plugin.svg
  icons/apps/22/system-help.svg
  icons/apps/22/virt-manager.svg
  icons/apps/32/hotspot.svg

To: ngraham, #vdg, ndavis
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27508: Remove unnecessary monochrome app icons

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D27508

To: ngraham, #vdg, ndavis
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27504: WIP: RFC: smb faster copy to local

[Nathaniel Graham]

ngraham added a comment.


  Yep I'm seeing pretty much double speed here. Amazing.
  
  This code is above my pay grade, so I won't try to review it. :) But the 
patch works as described!

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D27504

To: sitter, ngraham, cfeck
Cc: asturmlechner, kde-frameworks-devel, kfm-devel, pberestov, iasensio, 
fprice, LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D27296: [PlasmaComponents]Clean up ListItem code

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  cleanup (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27296

To: gvgeo, #plasma, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27083: [PC2]Give Label integer width.

[Nathaniel Graham]

ngraham added a comment.


  So the test needs to be fixed?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27083

To: gvgeo, #plasma, #vdg, ndavis, davidedmundson
Cc: ngraham, davidedmundson, kde-frameworks-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
michaelh, ZrenBot, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27460: fix layout size hints for button labels

[Nathaniel Graham]

ngraham added a comment.


  This patch also does not fix the media controller buttons' icon sizes, so 
D27449  is still needed. :/

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27460

To: mart, #plasma, davidedmundson
Cc: ngraham, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D27460: fix layout size hints for button labels

[Nathaniel Graham]

ngraham added a comment.


  The System Tray's pin button now looks like this for me:
  
  F8112931: Screenshot_20200219_083903.png 


REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27460

To: mart, #plasma, davidedmundson
Cc: ngraham, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D27504: WIP: RFC: smb faster copy to local

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> kio_smb_dir.cpp:88
> +const off_t idealSegmentSize = qMin(fileSize / 50,
> +4L * 1024 * 1024 /* 4 MiB */);
> +segmentSize = qMax(segmentSize,

This may actually be too optimistic. If you happen to use all 20 allowed 
instances of the slave (5 per server) and each copies a file that hits the 
segment limit, then we'd be looking at up to 400MiB. A bit steep. Then again, I 
am not sure how likely that is.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D27504

To: sitter, ngraham, cfeck
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov

D27504: WIP: RFC: smb faster copy to local

[Harald Sitter]

sitter created this revision.
sitter added reviewers: ngraham, cfeck.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  see https://bugs.kde.org/show_bug.cgi?id=291835#c57 for background
  
  - reading now happens inside a future. should be safe since we don't have any 
other threads doing anything while we wait.
  - the future feeds into a buffer from which the main thread will take file 
segments and write them to disk
  - buffer has 4 segments and synchronizes the threads via wait conditions
  - the size of a segment is determined somewhat dynamically between 64kb and 
4mb. the larger a file is the more it benefits from larger read requests
  
  under perfect conditions this yields approximately mount-level copy
  performance, unfortunately those are hard to hit so on average it's usually
  less (somewhere in the range of 10 to 20% depending on the actual file
  size and server type).
  
  compared to the old code this should more or less always be 100% faster for
  large files.
  
  multiple small files should see an equal improvement from what I've seen
  copying my kio-extras dir around.
  
  for some reason gains seem much greater against win10 servers, though that
  may be because the test systems I've set up are not having equivalent IO
  capabilities.
  
  in general, all of the above only applies to cases where the server's disk
  output will be able to saturate the request volume.
  
  TODO: this probably also needs extending to smbCopy (remote<->remote) and
  smbCopyPut (local->remote). smbcopy may need to not have threading though
  since we'd use the same smb_context in two threads

TEST PLAN
  - fallocate -l 1G file
  - copy around
  - copy kio-extras around

REPOSITORY
  R320 KIO Extras

BRANCH
  thread-read

REVISION DETAIL
  https://phabricator.kde.org/D27504

AFFECTED FILES
  smb/kio_smb_dir.cpp

To: sitter, ngraham, cfeck
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov

D27502: Create ConfigView an unmanaged ConfigWidget

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D27502

To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27502: Create ConfigView an unmanaged ConfigWidget

[Benjamin Port]

bport added a dependent revision: D27503: [KCM Spellchecking] port to 
KPropertySkeletonItem.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D27502

To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27502: Create ConfigView an unmanaged ConfigWidget

[Benjamin Port]

bport created this revision.
bport added reviewers: Plasma, ervin, crossi, meven.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bport requested review of this revision.

REVISION SUMMARY
  - Allow to manage controller externally
  - Compatible (for checkbox) with KConfigDialogManager (usage of kcfg_ in 
widget name)
  - Expose loader and settings in order to allow tham managed by a KCModule

TEST PLAN
  - automated test
  - Checked integration with Kate
  - ConfigView tested with a plasma-desktop patch

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D27502

AFFECTED FILES
  autotests/test_settings.cpp
  src/core/CMakeLists.txt
  src/core/guesslanguage.cpp
  src/core/languagefilter.cpp
  src/core/loader.cpp
  src/core/loader.h
  src/core/loader_p.h
  src/core/settings.cpp
  src/core/settings.h
  src/core/settings_p.h
  src/core/speller.cpp
  src/ui/CMakeLists.txt
  src/ui/configui.ui
  src/ui/configview.cpp
  src/ui/configview.h
  src/ui/configwidget.cpp
  src/ui/configwidget.h
  src/ui/dialog.cpp
  src/ui/highlighter.cpp

To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27334: Fix pri file: have qmake name of QtGui as dep, do not fail with CamelCase includes

[Friedrich W. H. Kossebau]

kossebau closed this revision.
kossebau added a comment.


  Pushed as eaab60ebc9835e0df5fc0c397dc72002a8870218 
 & 
0817e21427218cbb0fcff772f00b5d3e0e851939 


REPOSITORY
  R280 Prison

REVISION DETAIL
  https://phabricator.kde.org/D27334

To: kossebau, svuorela, vkrause, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27496: Fix isImmutable generated property

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R237:55aaa712b75b: Fix isImmutable 
generated property (authored by meven).

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27496?vs=75974&id=75995

REVISION DETAIL
  https://phabricator.kde.org/D27496

AFFECTED FILES
  autotests/kconfig_compiler/test13.h.ref
  src/kconfig_compiler/KConfigHeaderGenerator.cpp

To: meven, hchain, ervin, bport, crossi, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

Re: Fixing QNetworkAccessManager use

[Friedrich W. H. Kossebau]

Am Mittwoch, 19. Februar 2020, 08:05:01 CET schrieb Ben Cooksley:
> On Mon, Feb 3, 2020 at 7:42 AM Volker Krause  wrote:
> > It would also help to know where specifically we have that problem, so we
> > can actually solve it, and so we can figure out why we failed to fix this
> > there earlier.
> 
> Just bringing this up again - it seems we've not had much movement on
> this aside from the Wiki page.

The wiki page currently still just recommends to set
"networkAccessManger->setAttribute(QNetworkRequest::FollowRedirectsAttribute, 
true);"

Which seems simple, but possible not what is enough in all cases.

So my open questions here to be able to act on code I contribute to are:

a) What about the mentioned QNetworkRequest::NoLessSafeRedirectPolicy, in 
which cases should that be used and when not?

b) What about the HSTS stuff, when is that recommended?

c) What is a sane number for QNetworkRequest::maximumRedirectsAllowed?

Both in general and when it comes to KDE servers.

Personally I am still unsure what the actual issue is. Why are redirects 
needed at all. Why all the address changes all the time? The "U" in 
"URL"/"URI" is for "uniform", not "unstable", isn't it ;)

Can you give some examples for URLs of resources our code uses on KDE servers, 
and why they needed to change?

And if those redirects are permanent, should the client side not also 
permanently update to the new location then, instead of continuing to poke the 
old address every time again and again, until one day it will poke into a void 
because the backward compat redirect support has been dropped?

Cheers
Friedrich

Re: Banning QNetworkAccessManager

[Johnny Jazeix]

Hi,

as Volker said, without any alternative, we can't just ban QNAM.
And even with one, we would need time to implement it (for example,
for GCompris, this part of the code was written by someone who is less
active now, so rewriting it, testing it and be sure it works will take
some time).

Can't we use lxr to find all applications using it, check if they use
it in a good way or not instead?

Johnny


Le mer. 19 févr. 2020 à 10:05, Ben Cooksley  a écrit :
>
> On Wed, Feb 19, 2020 at 9:30 PM Volker Krause  wrote:
> >
> > On Wednesday, 19 February 2020 08:05:01 CET Ben Cooksley wrote:
> > > On Mon, Feb 3, 2020 at 7:42 AM Volker Krause  wrote:
> > > > I agree on the problem of QNAM's default, see also
> > > > https://conf.kde.org/en/
> > > > akademy2019/public/events/135 on that subject.
> > > >
> > > > On Saturday, 1 February 2020 23:24:14 CET Ben Cooksley wrote:
> > > > [...]
> > > >
> > > > > Prior to now, i've taken the approach of advertising that
> > > > > QNetworkAccessManager is broken and needs a flag set within
> > > > > applications when used, however unfortunately this seems to have been
> > > > > an ineffective approach and cases of broken behaviour continue to
> > > > > appear (despite this brokeness being known about and advertised since
> > > > > back in the Qt 4.x series when this class was introduced)
> > > > >
> > > > > Therefore, given the Qt Project is unlikely to change their position
> > > >
> > > > > on this, I would like to propose the following:
> > > > The Qt Project is actually in favor of changing at least the redirection
> > > > default to exactly what we want it to be.
> > > > https://codereview.qt-project.org/c/qt/qtbase/+/273175 has the change, 
> > > > and
> > > > got approval both by Lars and one of the maintainers. It is merely stuck
> > > > in dealing with the unit test fallout in Qt's CI that somebody has to
> > > > take care of. Doesn't immediately help us of course, but claiming Qt is
> > > > unwilling to change this is simply wrong.
> > > >
> > > > > 1) That effective immediately, QNetworkAccessManager and it's children
> > > > > classes be banned and prohibited within KDE software, to be enforced
> > > > > by server side hooks.
> > > > > 2) That we fork QNetworkAccessManager and the associated classes
> > > > > within the appropriate Framework (to be determined), where the
> > > > > defective behaviour can then be corrected.
> > > >
> > > > While this might solve the redirection problem, it brings us yet another
> > > > then unmaintained HTTP implementation next to the one in KIO, which is a
> > > > far bigger problem long term. We need less of those, not more.
> > > >
> > > > To address the problem of people not using the correct defaults, I did
> > > > propose a much less extreme approach in the above mentioned talk at
> > > > Akademy, namely having a KNetworkAccessManager as a sub-class of QNAM in
> > > > a low-tier frameworks that essentially just enables redirects and HSTS.
> > > > QNAM's implementation isn't the problem after all, the defaults are.
> > > >
> > > > Commit hooks warning about QNAM usage might still be a good idea, but 
> > > > as a
> > > > warning only. Hard blocking QNAM-using code would block at least three
> > > > repos I spend a lot of time on currently, none of which even talks to 
> > > > KDE
> > > > servers.
> > > >
> > > > If you need to take more drastic measures against code not doing this
> > > > correctly regardless, you can do that my dropping the server-side
> > > > workarounds. Yes, that would break the affected application possibly, 
> > > > but
> > > > at least it would not cause massive collateral damage for the people
> > > > using this correctly.
> > > >
> > > > It would also help to know where specifically we have that problem, so 
> > > > we
> > > > can actually solve it, and so we can figure out why we failed to fix 
> > > > this
> > > > there earlier.
> > >
> > > Just bringing this up again - it seems we've not had much movement on
> > > this aside from the Wiki page.
> > >
> > > Examining that Qt merge request, it not only is targeted at just Qt
> > > 6.x, it also hasn't had any movement since the start of October 2019 -
> > > 4 months ago.
> > > Given that we are going to be on Qt 5.x for some time, that merge
> > > request can't be considered to be the 'fix' for this issue.
> >
> > The targeting of Qt6 is due to this aiming at dev when that was still Qt 
> > 5.x,
> > but you are right of course, somebody needs to do the work there to drive 
> > this
> > to completion, no matter in which version it will actually land.
>
> Indeed.
>
> >
> > > We need a solution that will ensure software released today doesn't
> > > cause us pain in several years time, because as I illustrated in an
> > > earlier email to this thread, software has a habit of remaining in use
> > > for an extremely long amount of time (August 2014 being the release
> > > date of the Marble versions in question hitting that old URL).
> > >
> > > The easiest 

D27497: Fix code generation for entries with min/max

[Henri Chain]

hchain updated this revision to Diff 75987.
hchain added a comment.


  Add reference files for other tests that were impacted

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27497?vs=75985&id=75987

REVISION DETAIL
  https://phabricator.kde.org/D27497

AFFECTED FILES
  autotests/kconfig_compiler/CMakeLists.txt
  autotests/kconfig_compiler/kconfigcompiler_test.cpp
  autotests/kconfig_compiler/test13.cpp.ref
  autotests/kconfig_compiler/test_param_minmax.cpp.ref
  autotests/kconfig_compiler/test_param_minmax.h.ref
  autotests/kconfig_compiler/test_param_minmax.kcfg
  autotests/kconfig_compiler/test_param_minmax.kcfgc
  autotests/kconfig_compiler/test_param_minmax_main.cpp
  autotests/kconfig_compiler/test_properties_minmax.cpp.ref
  autotests/kconfig_compiler/test_properties_minmax.h.ref
  autotests/kconfig_compiler/test_properties_minmax.kcfg
  autotests/kconfig_compiler/test_properties_minmax.kcfgc
  autotests/kconfig_compiler/test_properties_minmax_main.cpp
  autotests/kconfig_compiler/test_signal.cpp.ref
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kconfig_compiler.cpp

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27497: Fix code generation for entries with min/max

[Henri Chain]

hchain updated this revision to Diff 75985.
hchain added a comment.


  Reduce number of lines in generated test code

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27497?vs=75983&id=75985

REVISION DETAIL
  https://phabricator.kde.org/D27497

AFFECTED FILES
  autotests/kconfig_compiler/CMakeLists.txt
  autotests/kconfig_compiler/kconfigcompiler_test.cpp
  autotests/kconfig_compiler/test_param_minmax.cpp.ref
  autotests/kconfig_compiler/test_param_minmax.h.ref
  autotests/kconfig_compiler/test_param_minmax.kcfg
  autotests/kconfig_compiler/test_param_minmax.kcfgc
  autotests/kconfig_compiler/test_param_minmax_main.cpp
  autotests/kconfig_compiler/test_properties_minmax.cpp.ref
  autotests/kconfig_compiler/test_properties_minmax.h.ref
  autotests/kconfig_compiler/test_properties_minmax.kcfg
  autotests/kconfig_compiler/test_properties_minmax.kcfgc
  autotests/kconfig_compiler/test_properties_minmax_main.cpp
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kconfig_compiler.cpp

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27496: Fix isImmutable generated property

[Cyril Rossi]

crossi accepted this revision.

REPOSITORY
  R237 KConfig

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27496

To: meven, hchain, ervin, bport, crossi, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27497: Fix code generation for entries with min/max

[Henri Chain]

hchain updated this revision to Diff 75983.
hchain added a comment.


  Yet more style fixes

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27497?vs=75982&id=75983

REVISION DETAIL
  https://phabricator.kde.org/D27497

AFFECTED FILES
  autotests/kconfig_compiler/CMakeLists.txt
  autotests/kconfig_compiler/kconfigcompiler_test.cpp
  autotests/kconfig_compiler/test_param_minmax.cpp.ref
  autotests/kconfig_compiler/test_param_minmax.h.ref
  autotests/kconfig_compiler/test_param_minmax.kcfg
  autotests/kconfig_compiler/test_param_minmax.kcfgc
  autotests/kconfig_compiler/test_param_minmax_main.cpp
  autotests/kconfig_compiler/test_properties_minmax.cpp.ref
  autotests/kconfig_compiler/test_properties_minmax.h.ref
  autotests/kconfig_compiler/test_properties_minmax.kcfg
  autotests/kconfig_compiler/test_properties_minmax.kcfgc
  autotests/kconfig_compiler/test_properties_minmax_main.cpp
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kconfig_compiler.cpp

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27497: Fix code generation for entries with min/max

[Henri Chain]

hchain updated this revision to Diff 75982.
hchain added a comment.


  Fix Style/whitespace

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27497?vs=75978&id=75982

REVISION DETAIL
  https://phabricator.kde.org/D27497

AFFECTED FILES
  autotests/kconfig_compiler/CMakeLists.txt
  autotests/kconfig_compiler/kconfigcompiler_test.cpp
  autotests/kconfig_compiler/test_param_minmax.cpp.ref
  autotests/kconfig_compiler/test_param_minmax.h.ref
  autotests/kconfig_compiler/test_param_minmax.kcfg
  autotests/kconfig_compiler/test_param_minmax.kcfgc
  autotests/kconfig_compiler/test_param_minmax_main.cpp
  autotests/kconfig_compiler/test_properties_minmax.cpp.ref
  autotests/kconfig_compiler/test_properties_minmax.h.ref
  autotests/kconfig_compiler/test_properties_minmax.kcfg
  autotests/kconfig_compiler/test_properties_minmax.kcfgc
  autotests/kconfig_compiler/test_properties_minmax_main.cpp
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kconfig_compiler.cpp

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27296: [PlasmaComponents]Clean up ListItem code

[George Vogiatzis]

gvgeo added a comment.


  The 2 removed lines were leftovers from 54e87fe879ba 
.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27296

To: gvgeo, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27497: Fix code generation for entries with min/max

[Benjamin Port]

bport added inline comments.

INLINE COMMENTS

> test_param_minmax.kcfg:17
> +
> \ No newline at end of file


Add a new line

> test_param_minmax.kcfgc:4
> +Mutators=true
> \ No newline at end of file


add a new line

> test_param_minmax_main.cpp:28
> +return 0;
> +}

Missing new line at end of this file

> KConfigCommonStructs.h:185
> +QString newItemInner(
> +const CfgEntry* entry,
> +const QString &key,

Space before * not after

> kconfig_compiler.cpp:478
> +
> +QString newItem(const CfgEntry* entry, const QString &key, const QString& 
> defaultValue,
> +const KConfigParameters &cfg, const QString ¶m) {

Space before * not after

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D27497

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27496: Fix isImmutable generated property

[Henri Chain]

hchain accepted this revision.
hchain added a comment.
This revision is now accepted and ready to land.


  LGTM

REPOSITORY
  R237 KConfig

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27496

To: meven, hchain, ervin, bport, crossi, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27497: Fix code generation for entries with min/max

[Henri Chain]

hchain created this revision.
hchain added reviewers: meven, crossi, ervin, bport, tcanabrava.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
hchain requested review of this revision.

REVISION SUMMARY
  - When GenerateProperties and Mutators are activated, the generated code did 
not handle min/max properly
  
  - In the case of a parameterized entry, generated code also did not handle 
min/max

TEST PLAN
  - auto tests included

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D27497

AFFECTED FILES
  autotests/kconfig_compiler/CMakeLists.txt
  autotests/kconfig_compiler/kconfigcompiler_test.cpp
  autotests/kconfig_compiler/test_param_minmax.cpp.ref
  autotests/kconfig_compiler/test_param_minmax.h.ref
  autotests/kconfig_compiler/test_param_minmax.kcfg
  autotests/kconfig_compiler/test_param_minmax.kcfgc
  autotests/kconfig_compiler/test_param_minmax_main.cpp
  autotests/kconfig_compiler/test_properties_minmax.cpp.ref
  autotests/kconfig_compiler/test_properties_minmax.h.ref
  autotests/kconfig_compiler/test_properties_minmax.kcfg
  autotests/kconfig_compiler/test_properties_minmax.kcfgc
  autotests/kconfig_compiler/test_properties_minmax_main.cpp
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kconfig_compiler.cpp

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27083: [PC2]Give Label integer width.

[George Vogiatzis]

gvgeo abandoned this revision.
gvgeo added a comment.


  The problem is with controls, not with label. QtQuick controls work fine.
  
  problem
  F8112471: Screenshot_20200219_124506.png 
 F8112472: Screenshot_20200219_124602.png 

  
  Proper icon display with label width real. (PC2 with this patch)
  F8112469: Screenshot_20200219_124730.png 
 F8112470: Screenshot_20200219_125928.png 

  
  F8112477: test.qml 

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27083

To: gvgeo, #plasma, #vdg, ndavis, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, michaelh, ZrenBot, 
ngraham, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D27460: fix layout size hints for button labels

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:ba0b637de94a: fix layout size hints for button labels 
(authored by mart).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27460?vs=75976&id=75977

REVISION DETAIL
  https://phabricator.kde.org/D27460

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/Button.qml
  src/declarativeimports/plasmacomponents3/TabButton.qml
  src/declarativeimports/plasmacomponents3/TextField.qml
  src/declarativeimports/plasmacomponents3/ToolButton.qml
  tests/components/button3.qml
  tests/components/tabbar3.qml
  tests/components/toolbutton3.qml

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27460: fix layout size hints for button labels

[David Edmundson]

davidedmundson accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  phab/buttonslayout

REVISION DETAIL
  https://phabricator.kde.org/D27460

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27460: fix layout size hints for button labels

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in Button.qml:62
> I don't understand this part about constraining the width to the parent 
> height  (for all 3)
> 
> If I remove it all the tests still pass.

was an extra constraint to have buttons always square, but i added some test 
and removed this , they seem to behave always well without it

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27460

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27460: fix layout size hints for button labels

[Marco Martin]

mart updated this revision to Diff 75976.
mart marked an inline comment as done.
mart added a comment.


  - remove extra check for squareness

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27460?vs=75972&id=75976

BRANCH
  phab/buttonslayout

REVISION DETAIL
  https://phabricator.kde.org/D27460

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/Button.qml
  src/declarativeimports/plasmacomponents3/TabButton.qml
  src/declarativeimports/plasmacomponents3/TextField.qml
  src/declarativeimports/plasmacomponents3/ToolButton.qml
  tests/components/button3.qml
  tests/components/tabbar3.qml
  tests/components/toolbutton3.qml

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27496: Fix isImmutable generated property

[Méven Car]

meven created this revision.
meven added reviewers: hchain, ervin, bport, crossi, Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Relates to D26368 

TEST PLAN
  ctest

REPOSITORY
  R237 KConfig

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27496

AFFECTED FILES
  autotests/kconfig_compiler/test13.h.ref
  src/kconfig_compiler/KConfigHeaderGenerator.cpp

To: meven, hchain, ervin, bport, crossi, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27460: fix layout size hints for button labels

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> Button.qml:62
> +Layout.minimumHeight: units.iconSizes.tiny
> +Layout.maximumWidth: Math.min(parent.height, control.icon.width 
> > 0 ? control.icon.width : units.iconSizes.small)
> +Layout.maximumHeight: Math.min(parent.width, control.icon.height 
> > 0 ? control.icon.height : units.iconSizes.small)

I don't understand this part about constraining the width to the parent height  
(for all 3)

If I remove it all the tests still pass.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27460

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27460: fix layout size hints for button labels

[Marco Martin]

mart marked an inline comment as done.
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in button3.qml:90
> You didn't even run this :(
> 
> 1. icon.name not icon.source
> 2. now the comment next to it is wrong
> 3. there's a binding loop
> 
> 4. if I make a test without any text, which is what the thing you're 
> supposedly fixing was doing, I don't get an icon

sorry,
tested, fixed the problem and added new tests

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27460

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27460: fix layout size hints for button labels

[Marco Martin]

mart updated this revision to Diff 75972.
mart marked an inline comment as done.
mart added a comment.


  - fix button display property
  - add a test for tabbar

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27460?vs=75927&id=75972

BRANCH
  phab/buttonslayout

REVISION DETAIL
  https://phabricator.kde.org/D27460

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/Button.qml
  src/declarativeimports/plasmacomponents3/TabButton.qml
  src/declarativeimports/plasmacomponents3/TextField.qml
  src/declarativeimports/plasmacomponents3/ToolButton.qml
  tests/components/button3.qml
  tests/components/tabbar3.qml
  tests/components/toolbutton3.qml

To: mart, #plasma, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[Méven Car]

meven updated this revision to Diff 75969.
meven added a comment.


  Add a since to dox

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27463?vs=75968&id=75969

BRANCH
  arcpatch-D27463

REVISION DETAIL
  https://phabricator.kde.org/D27463

AFFECTED FILES
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test4.kcfg
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/README.dox
  src/kconfig_compiler/kcfg.xsd

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[Méven Car]

meven updated this revision to Diff 75968.
meven added a comment.


  Add documentation about value in dox

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27463?vs=75967&id=75968

BRANCH
  arcpatch-D27463

REVISION DETAIL
  https://phabricator.kde.org/D27463

AFFECTED FILES
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test4.kcfg
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/README.dox
  src/kconfig_compiler/kcfg.xsd

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

Re: Banning QNetworkAccessManager

[Ben Cooksley]

On Wed, Feb 19, 2020 at 9:30 PM Volker Krause  wrote:
>
> On Wednesday, 19 February 2020 08:05:01 CET Ben Cooksley wrote:
> > On Mon, Feb 3, 2020 at 7:42 AM Volker Krause  wrote:
> > > I agree on the problem of QNAM's default, see also
> > > https://conf.kde.org/en/
> > > akademy2019/public/events/135 on that subject.
> > >
> > > On Saturday, 1 February 2020 23:24:14 CET Ben Cooksley wrote:
> > > [...]
> > >
> > > > Prior to now, i've taken the approach of advertising that
> > > > QNetworkAccessManager is broken and needs a flag set within
> > > > applications when used, however unfortunately this seems to have been
> > > > an ineffective approach and cases of broken behaviour continue to
> > > > appear (despite this brokeness being known about and advertised since
> > > > back in the Qt 4.x series when this class was introduced)
> > > >
> > > > Therefore, given the Qt Project is unlikely to change their position
> > >
> > > > on this, I would like to propose the following:
> > > The Qt Project is actually in favor of changing at least the redirection
> > > default to exactly what we want it to be.
> > > https://codereview.qt-project.org/c/qt/qtbase/+/273175 has the change, and
> > > got approval both by Lars and one of the maintainers. It is merely stuck
> > > in dealing with the unit test fallout in Qt's CI that somebody has to
> > > take care of. Doesn't immediately help us of course, but claiming Qt is
> > > unwilling to change this is simply wrong.
> > >
> > > > 1) That effective immediately, QNetworkAccessManager and it's children
> > > > classes be banned and prohibited within KDE software, to be enforced
> > > > by server side hooks.
> > > > 2) That we fork QNetworkAccessManager and the associated classes
> > > > within the appropriate Framework (to be determined), where the
> > > > defective behaviour can then be corrected.
> > >
> > > While this might solve the redirection problem, it brings us yet another
> > > then unmaintained HTTP implementation next to the one in KIO, which is a
> > > far bigger problem long term. We need less of those, not more.
> > >
> > > To address the problem of people not using the correct defaults, I did
> > > propose a much less extreme approach in the above mentioned talk at
> > > Akademy, namely having a KNetworkAccessManager as a sub-class of QNAM in
> > > a low-tier frameworks that essentially just enables redirects and HSTS.
> > > QNAM's implementation isn't the problem after all, the defaults are.
> > >
> > > Commit hooks warning about QNAM usage might still be a good idea, but as a
> > > warning only. Hard blocking QNAM-using code would block at least three
> > > repos I spend a lot of time on currently, none of which even talks to KDE
> > > servers.
> > >
> > > If you need to take more drastic measures against code not doing this
> > > correctly regardless, you can do that my dropping the server-side
> > > workarounds. Yes, that would break the affected application possibly, but
> > > at least it would not cause massive collateral damage for the people
> > > using this correctly.
> > >
> > > It would also help to know where specifically we have that problem, so we
> > > can actually solve it, and so we can figure out why we failed to fix this
> > > there earlier.
> >
> > Just bringing this up again - it seems we've not had much movement on
> > this aside from the Wiki page.
> >
> > Examining that Qt merge request, it not only is targeted at just Qt
> > 6.x, it also hasn't had any movement since the start of October 2019 -
> > 4 months ago.
> > Given that we are going to be on Qt 5.x for some time, that merge
> > request can't be considered to be the 'fix' for this issue.
>
> The targeting of Qt6 is due to this aiming at dev when that was still Qt 5.x,
> but you are right of course, somebody needs to do the work there to drive this
> to completion, no matter in which version it will actually land.

Indeed.

>
> > We need a solution that will ensure software released today doesn't
> > cause us pain in several years time, because as I illustrated in an
> > earlier email to this thread, software has a habit of remaining in use
> > for an extremely long amount of time (August 2014 being the release
> > date of the Marble versions in question hitting that old URL).
> >
> > The easiest path forward is to simply ban QNetworkAccessManager.
>
> That might be the easiest path for you, but certainly not for me, given two of
> the modules I work on most atm are using QNAM (not even to talk to KDE
> infrastructure), and would suddenly no longer be allowed to be hosted here?

Ideally they would move to whatever replacement we have for QNAM
should it be banned :)

>
> Also, I have yet to see a suggestion for a viable alternative to QNAM. The
> initially proposed fork would mean trading the possibility of broken redirect
> handling for an unmaintained HTTP stack, I don't think that's a good deal for
> the security of our users.
>

It isn't perfect yes.

Sadly however the nega

D27463: KconfigXT: Add a value attribute to Enum field choices

[Méven Car]

meven added a comment.


  In D27463#613961 , @davidre wrote:
  
  > Could you also add documentation to whatever  
https://api.kde.org/frameworks/kconfig/html/kconfig_compiler.html is generated 
from?
  
  
  Thanks for pointing it out.
  
  I meant to edit 
https://techbase.kde.org/Development/Tutorials/Using_KConfig_XT as well after 
merge.

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D27463

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[David Redondo]

davidre added a comment.


  Could you also add documentation to whatever  
https://api.kde.org/frameworks/kconfig/html/kconfig_compiler.html is generated 
from?

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D27463

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[Méven Car]

meven updated this revision to Diff 75967.
meven added a comment.


  Ignore empty value attribute for choice

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27463?vs=75966&id=75967

BRANCH
  arcpatch-D27463

REVISION DETAIL
  https://phabricator.kde.org/D27463

AFFECTED FILES
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test4.kcfg
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kcfg.xsd

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[Méven Car]

meven updated this revision to Diff 75966.
meven added a comment.


  Default value must refere to the choice name

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27463?vs=75964&id=75966

BRANCH
  arcpatch-D27463

REVISION DETAIL
  https://phabricator.kde.org/D27463

AFFECTED FILES
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test4.kcfg
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kcfg.xsd

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

Re: Banning QNetworkAccessManager

[Volker Krause]

On Wednesday, 19 February 2020 08:05:01 CET Ben Cooksley wrote:
> On Mon, Feb 3, 2020 at 7:42 AM Volker Krause  wrote:
> > I agree on the problem of QNAM's default, see also
> > https://conf.kde.org/en/
> > akademy2019/public/events/135 on that subject.
> > 
> > On Saturday, 1 February 2020 23:24:14 CET Ben Cooksley wrote:
> > [...]
> > 
> > > Prior to now, i've taken the approach of advertising that
> > > QNetworkAccessManager is broken and needs a flag set within
> > > applications when used, however unfortunately this seems to have been
> > > an ineffective approach and cases of broken behaviour continue to
> > > appear (despite this brokeness being known about and advertised since
> > > back in the Qt 4.x series when this class was introduced)
> > > 
> > > Therefore, given the Qt Project is unlikely to change their position
> > 
> > > on this, I would like to propose the following:
> > The Qt Project is actually in favor of changing at least the redirection
> > default to exactly what we want it to be.
> > https://codereview.qt-project.org/c/qt/qtbase/+/273175 has the change, and
> > got approval both by Lars and one of the maintainers. It is merely stuck
> > in dealing with the unit test fallout in Qt's CI that somebody has to
> > take care of. Doesn't immediately help us of course, but claiming Qt is
> > unwilling to change this is simply wrong.
> > 
> > > 1) That effective immediately, QNetworkAccessManager and it's children
> > > classes be banned and prohibited within KDE software, to be enforced
> > > by server side hooks.
> > > 2) That we fork QNetworkAccessManager and the associated classes
> > > within the appropriate Framework (to be determined), where the
> > > defective behaviour can then be corrected.
> > 
> > While this might solve the redirection problem, it brings us yet another
> > then unmaintained HTTP implementation next to the one in KIO, which is a
> > far bigger problem long term. We need less of those, not more.
> > 
> > To address the problem of people not using the correct defaults, I did
> > propose a much less extreme approach in the above mentioned talk at
> > Akademy, namely having a KNetworkAccessManager as a sub-class of QNAM in
> > a low-tier frameworks that essentially just enables redirects and HSTS.
> > QNAM's implementation isn't the problem after all, the defaults are.
> > 
> > Commit hooks warning about QNAM usage might still be a good idea, but as a
> > warning only. Hard blocking QNAM-using code would block at least three
> > repos I spend a lot of time on currently, none of which even talks to KDE
> > servers.
> > 
> > If you need to take more drastic measures against code not doing this
> > correctly regardless, you can do that my dropping the server-side
> > workarounds. Yes, that would break the affected application possibly, but
> > at least it would not cause massive collateral damage for the people
> > using this correctly.
> > 
> > It would also help to know where specifically we have that problem, so we
> > can actually solve it, and so we can figure out why we failed to fix this
> > there earlier.
> 
> Just bringing this up again - it seems we've not had much movement on
> this aside from the Wiki page.
> 
> Examining that Qt merge request, it not only is targeted at just Qt
> 6.x, it also hasn't had any movement since the start of October 2019 -
> 4 months ago.
> Given that we are going to be on Qt 5.x for some time, that merge
> request can't be considered to be the 'fix' for this issue.

The targeting of Qt6 is due to this aiming at dev when that was still Qt 5.x, 
but you are right of course, somebody needs to do the work there to drive this 
to completion, no matter in which version it will actually land.

> We need a solution that will ensure software released today doesn't
> cause us pain in several years time, because as I illustrated in an
> earlier email to this thread, software has a habit of remaining in use
> for an extremely long amount of time (August 2014 being the release
> date of the Marble versions in question hitting that old URL).
> 
> The easiest path forward is to simply ban QNetworkAccessManager.

That might be the easiest path for you, but certainly not for me, given two of 
the modules I work on most atm are using QNAM (not even to talk to KDE 
infrastructure), and would suddenly no longer be allowed to be hosted here?

Also, I have yet to see a suggestion for a viable alternative to QNAM. The 
initially proposed fork would mean trading the possibility of broken redirect 
handling for an unmaintained HTTP stack, I don't think that's a good deal for 
the security of our users.

Instead, how about the way we approached this for KUserFeedback? Before 
getting things deployed on KDE infrastructure for use by library or 
application code, sysadmins and developers review if the implementation is 
allowing easy and secure operation of the infrastructure, redirect handling 
being one part of this. In case of KUserFeedback you suggest

D27463: KconfigXT: Add a value attribute to Enum field choices

[Méven Car]

meven updated this revision to Diff 75964.
meven marked 2 inline comments as done.
meven added a comment.


  Clean garbage files

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27463?vs=75917&id=75964

BRANCH
  arcpatch-D27463

REVISION DETAIL
  https://phabricator.kde.org/D27463

AFFECTED FILES
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test4.kcfg
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kcfg.xsd

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns