[effects] Port to new connect syntax
ClosedPublic

Authored by zzag on Jan 18 2019, 7:14 PM.

Details

Summary

The new connect syntax has several advantages over the old syntax:

(a) Connecting with the new syntax is faster;
(b) It is compile time checked.

There are still a few places where the old connect syntax is used, e.g.
connecting to QML buttons in the Desktop Grid effect.

Test Plan

Have been testing this patch for ~2 weeks, haven't noticed any
regressions.

Diff Detail

Repository
R108 KWin
Branch
effects-port-to-new-connect-syntax
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7281
Build 7299: arc lint + arc unit
zzag created this revision.Jan 18 2019, 7:14 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 18 2019, 7:14 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jan 18 2019, 7:14 PM
zzag updated this revision to Diff 49835.Jan 18 2019, 7:19 PM

Make arc linter happier

zzag added a comment.EditedJan 18 2019, 7:34 PM

I know it's a huge patch, but I hope it would be okay to merge it. We have a big time frame to test the patch.

Given that it compiles, it looks fine to me

broulik added inline comments.
effects/desktopgrid/desktopgrid_config.cpp
89

Do we allow C++14?

zzag added inline comments.Jan 21 2019, 10:20 AM
effects/desktopgrid/desktopgrid_config.cpp
89
zzag added a comment.Jan 23 2019, 10:33 AM

Given that it compiles, it looks fine to me

... speaking of hand porting and human errors, I don't think it would be a good idea to use a script or something like that to port KWin to the new connect syntax because there are cases when we still want to use the old syntax.

davidedmundson accepted this revision.EditedJan 25 2019, 10:42 PM
davidedmundson added a subscriber: davidedmundson.

Biggest issue I've seen with this connect porting is when people use slots as fake-virtuals.
Something to be wary of in the core, but in here it's all leaf classes.

Given Martin's happy too, go for it.

This revision is now accepted and ready to land.Jan 25 2019, 10:42 PM
zzag added a comment.Jan 25 2019, 10:47 PM

Biggest I've seen with this connect porting is when people use slots as fake-virtuals.

Can you please explain what fake-virtuals are?

If you have:

Base
{
public Q_SLOTS:
void foo();
}

Derived
{
public Q_SLOTS:
void foo();
}

asdf = new Derived();
connect(.., asdf, SLOT(foo())); goes to Derived
connect(..., asdf, &Base::foo); goes to Base

This revision was automatically updated to reflect the committed changes.