Review Request 126895: Make KGlobalAccel dependency in KXmlGui optional

[Andre Heinecke]

Hi,

On Wednesday 27 January 2016 21:28:36 Andreas Hartmetz wrote:
> On Mittwoch, 27. Januar 2016 17:21:33 CET Boudewijn Rempt wrote:
> [snip]
> 
> > whether kglobalshortcuts is functional or not is besides the point:
> > the point is that whether it works or not it doesn't add any
> > functionality to the average application. Global shortcuts are useful
> > only for a very limited set of applications, so it should always have
> > been an optional dependency.
> 
> Here is something that is never beside the point:
> maintenance burden and continuous-integration-ability.
> ifdefs are somewhat bad for maintainability and really, really bad for a
> continuous integration systems's ability to detect relevant build
> breakage. I am repeating Martin's argument here with different words.

To repeat our argument (yay) we feel that we need to have the KXmlGui library 
as one of the core libraries for "KDE ishness" not to always depend on 
KGlobalAccel on all platforms as other Platforms have different concepts for 
Global shortcuts and they don't make sense for our users that we see.

Wrt maintenance burden / CI effort. Please you can say you only maintain and 
Test the recommended usage. Our alternative is that we have to Patch it out 
then we have to maintain the patches and basically a fork.

Also this is really a very simple patch. It's not much different behavior it 
just does not allow you to configure Global Shortcuts through KxmlGui.

> The most maintainable way to make a feature optional is to disable it at
> runtime. 

I disagree with that. Runtime dependencies are more likely to cause bugs as a 
developer can not easily see / know how a function behaves when writing code.

Runtime dependencies also don't make sense for single Application deployments 
as we control all dependencies. The way it was in the past users had a Global 
Shortcuts configuration entry through KXmlGui which was always dysfunctional 
because we did not ship the necessary runtime components. 

And KGlobalAccel is not the only thing. I don't want to see spellchecking 
entires either that won't work because we have not installed a spellchecking 
libary or dictionaries. Etc.

> Only when this severely hurts performance or a feature plain
> won't build on a certain platform should it be disabled via ifdefs, and
> in the latter case, maybe it can be put into its own class that gets a
> real and a dummy implementation.

I dislike the dummy implementation. This makes code harder to understand and 
is more effort to maintain then simple harsh ifdefs.

It's also not a pattern already established for optional library depdenencies. 
It's established in the form of different implementations which might be 
dummies in a single library for different platforms. But to have dummy classes 
for optional dependencies is something different and afaik not established in 
KDE.

> My argument is first and foremost but for putting the knife where the
> least amount of stuff needs to be cut,

I think that XMLGui is the right place for this. XMLGui is a useful library 
without Global Shortcuts. It's very little code that needs GlobalAccel and it 
was easy to cut it out. There were not even any header / API changes 
neccessary for this.

> even if that results in shipping
> an unnecessary (small!) module.

I fail to see how this would be advantageous at all. Having code in a library 
that allows it to be dysfunctional is wrong. And I would not propose making 
such a change in GlobalAccel. Why should anyone would want to waste resources 
for something that does not add anything at all.

Shipping a library means that we have to include it in our build system, we 
have to update it regularly, we have to fix build errors for our platform, we 
have to ship the license, we have to include the source code in our source 
distribution etc. 

> Maybe there is a better solution, though.
> How about a dummy KGlobalShortcuts implementation in KXmlGui? There may
> be a few places where ifdefs are still required (e.g. completely hiding
> - and I don't mean removing - inert UI), but the vast majority of things
> in KGlobalShortcuts can be faked very easily by doing nothing at all.
> Most code paths don't need to be removed if there is simply no way to
> trigger them.

See above. A dummy library would imho be a more invasive change and harder to 
maintain then the ifdefs.

> The thing to be faked is a system that has no global shortcuts and where
> setting them somehow always fails. Maybe pretend to applications that it
> doesn't fail, shouldn't really matter.

Why would you want to do that? The only thing it adds is confusion for 
developers and users imho.

Regards,
Andre

-- 
Andre Heinecke |  ++49-541-335083-262  | http://www.intevation.de/
Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998
Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 648 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160128/96372a5d/attachment-0001.sig>