Differential D12228

kdirlister new connect syntax
ClosedPublic
Actions

Authored by jtamate on Apr 15 2018, 4:49 PM.

Details

Reviewers
dfaure
Group Reviewers
Frameworks
Commits
R241:ee0a0f1ef323: kdirlister new connect syntax
Summary

Convert to the new connect syntax.

Test Plan

Compiles and the tests pass.

Diff Detail

Repository
R241 KIO
Lint
Lint Skipped
Unit
Unit Tests Skipped
jtamate created this revision.Apr 15 2018, 4:49 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 15 2018, 4:49 PM
jtamate requested review of this revision.Apr 15 2018, 4:49 PM
dfaure added a comment.Apr 16 2018, 7:37 AM

Signals having changed from protected to public in Qt5, the whole idea of preventing others to emit signals went down the drain anyway. Extending that to KJob we could make the signals non-private there, with strong documentation "don't emit this yourself".

jtamate updated this revision to Diff 32362.Apr 17 2018, 6:36 AM
jtamate edited the summary of this revision. (Show Details)
jtamate edited the test plan for this revision. (Show Details)

I have renamed one private slot because I didn't find a better solution.
QOverload seems to work only for signals, not for slots.

jtamate edited the summary of this revision. (Show Details)Apr 17 2018, 6:55 AM
jtamate updated this revision to Diff 32656.Apr 20 2018, 5:09 PM
jtamate edited the summary of this revision. (Show Details)
jtamate edited the test plan for this revision. (Show Details)

Change the first connect.
No need to change the name of the slot.
Pass the plain error message.

dfaure requested changes to this revision.Apr 20 2018, 5:14 PM

Looks good, but shouldn't you remove the Q_PRIVATE_SLOT declarations too? They aren't used anymore, AFAICS.

src/core/kcoredirlister.cpp
2720 ↗(On Diff #32195)

missing space after the first comma

This revision now requires changes to proceed.Apr 20 2018, 5:14 PM
jtamate updated this revision to Diff 32658.Apr 20 2018, 5:22 PM

Added space.
Removed SLOT.

dfaure added a comment.Apr 20 2018, 5:23 PM

I meant this stuff:

src/core/kcoredirlister.h: Q_PRIVATE_SLOT(d, void _k_slotPercent(KJob *, unsigned long))

jtamate updated this revision to Diff 32661.Apr 20 2018, 5:37 PM

Removed Q_PRIVATE_SLOT. Didn't saw them again. :-(

dfaure added inline comments.Apr 20 2018, 5:55 PM
src/core/kcoredirlister.cpp
525 ↗(On Diff #32195)

This should probably not be here ;)

jtamate updated this revision to Diff 32663.Apr 20 2018, 6:07 PM

Removed comment.

dfaure accepted this revision.Apr 20 2018, 8:40 PM
This revision is now accepted and ready to land.Apr 20 2018, 8:40 PM
Closed by commit R241:ee0a0f1ef323: kdirlister new connect syntax (authored by jtamate). · Explain WhyApr 21 2018, 7:13 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 32362

View Options

src/core/kcoredirlister.cpp

Loading...
View Options

src/core/kcoredirlister_p.h

Loading...