See https://invent.kde.org/frameworks/knewstuff/-/merge_requests/74/ for a deprecation MR of KNS3::standardActionUpload, this method is only used in parley and the functionality is defunct anyway.

KNS3::standardAction is still used in a variety of places, but they are relatively easy to port.

After the MRs land the task depends only on the KF6-Branching.

https://invent.kde.org/frameworks/knewstuff/-/merge_requests/74
https://invent.kde.org/education/parley/-/merge_requests/5
https://invent.kde.org/education/marble/-/merge_requests/34
https://invent.kde.org/education/kwordquiz/-/merge_requests/4
https://invent.kde.org/education/kstars/-/merge_requests/145
https://invent.kde.org/pim/grantlee-editor/-/merge_requests/3

Can someone give me a rationale for this?

I generally think it's a bad idea to get rid of shared code and spawn it to multiple places.

Can someone give me a rationale for this?

As the description states this uses the KActionCollection which should be avoided.

Also this is one of the ancient stuff that is lying around in KNS. This method is just creating an action, connecting a signal and adds it to the action collection, so we can easily avoid using it. And method forces us to use the Qt4-style connects, which is sth. we should avoid doing.

Mostly the method had nothing specific to do with KNS, except the little icon we set in the action, so it makes little sense to share ;)

except the little icon we set in the action, so it makes little sense to share ;)

I disagree, we don't want to change that icon in 53 different places when someone decides they want to use a different name.

Also what's wrong with KActionCollection?

I disagree, we don't want to change that icon in 53 different places when someone decides they want to use a different name.

There you have confirmed that the icon is widely used outside of this method ;) The method is only called in a handful of places.

And it /really/ does not make sense to put logic like that in KNS, it enforces an deprecated connect syntax and is not flexible. And as you can see with the usage of the icon the alternatives(doing it manually) is by far more popular in the KDE codebase.

So if we want to address your issues we would need to export the KNS icon individually and not in some weird method.

I don't understand why you say this is a weird method, it's exactly what the KStandardAction methods so, and they're used *everywhere*

Are you suggestion we should get rid of KStandardAction::open too?

https://lxr.kde.org/search?_filestring=&_string=KStandardAction%3A%3Aopen(

After all it's like just a few lines to create the action and set the icon.

If your problem is the deprecated syntax, why not fix that to support the modern syntax?

As background, back in the last KF6 sprint (december 2019, before the world ended) it was done a big review of each framework in order to untangle the dependency tree as much as possible (and remove dependencies between frameworks, if possible making them rise in tiers. (this task is part of the kf6 workboard)

The main think keeping this framework dependent from KXmlGui (and most of its dependencies in turn is KActionCollection, so there was decided to try to get rid of that dependency.

No need to throw away the whole thing, it could just return an action without the standardaction and allow slots via template. In summary move to KStandardAction api.

Or even maybe add the action to KStandardAction, given that you need to hook it up manually either way?

A simple solution here would be to turn this into a factory method that simply returns a new QAction and then leaves it up to the caller to do the connections separately. That would remove the need for old-style connects while still having a "standard" action function. That said, if the only concern of that function of that function is the icon name I don't think it's very useful.

I disagree, we don't want to change that icon in 53 different places when someone decides they want to use a different name.

This is not unique to the ghns icon though, this would apply to many different icons. I'd personally consider the icon name to be public API and thus you shouldn't just suddenly change that.

In T12200#245725, @mart wrote:

As background, back in the last KF6 sprint (december 2019, before the world ended) it was done a big review of each framework in order to untangle the dependency tree as much as possible (and remove dependencies between frameworks, if possible making them rise in tiers. (this task is part of the kf6 workboard)

The main think keeping this framework dependent from KXmlGui (and most of its dependencies in turn is KActionCollection, so there was decided to try to get rid of that dependency.

I understand the aversion from KXmlGUi comes from wanting to somehow use this in QML (because from all the users you removed this function call, all of them use KXmlGUI anyway).

Maybe what you want is to split the library, not remove functionality that is being used and makes sense to be shared?

Or even maybe add the action to KStandardAction, given that you need to hook it up manually either way?

That would be a bit of a layering violation, KStandardAction is for standard actions, and i'd say KNS isn't really "very standard", but sure, that'd be an option

In T12200#245744, @aacid wrote:

Or even maybe add the action to KStandardAction, given that you need to hook it up manually either way?

That would be a bit of a layering violation, KStandardAction is for standard actions, and i'd say KNS isn't really "very standard", but sure, that'd be an option

Fair enough. In my mind a bunch of KDE apps have the Get New Thing so semi standard?

Yeah, i guess it can be considered semi standard.

On the other hand, most of the MRs where approved just fine so i guess maintainers don't feel as strongly as i do about code duplication ^_^

The standard way to do stuff with KNS is to use the Buttons, both for QWidgets and QML available.

I also want to mention that the QWidgets UI Elements from KNS are on maintenance mode (just bugs are fixed, no more UI improvements/features). The only reason that they are not formally deprecated is because there is no alternative, other than the QML parts.

What @davidre suggested with a factory method would be worth keeping in mind when we come up with a solution for the users of the QWidgets components :)

We have in KNSWidgets an Action class, which internally takes care of the KNS logic.
The standard action stuff has been removed for master.