Make member variable names, & placement and * placement more coherent
ClosedPublic
Actions

Authored by jeanv on Aug 14 2017, 4:50 PM.

Details

Reviewers

nicolasfella
albertvaka

Group Reviewers

KDE Connect

Commits

R224:71a4c5b6d9bf: Make member variable names, & placement and * placement more coherent
R224:72535ecf48db: Make member variable names, & placement and * placement more coherent

Summary

Change all member variables to the form m_fooBar because it is the preferred form in Qt (it was half and half between this and mFooBar, and a minority didn't have anything).
Place all references and pointers on the side of the type since it is the majority.

Basically:

mFoo -> m_foo
foo -> m_foo (if it is a member variable)
Type &ref -> Type& ref
Type *ptr -> Type* ptr

Diff Detail

Repository

R224 KDE Connect

Lint

Lint Skipped

Unit

Unit Tests Skipped

jeanv created this revision.Aug 14 2017, 4:50 PM

Include double pointers (Type **foo -> Type** foo)

nicolasfella accepted this revision.Aug 15 2017, 10:16 PM

This revision is now accepted and ready to land.Aug 15 2017, 10:16 PM

I see some inconsistencies, for example in DaemonPrivate you changed
QSet<LinkProvider*> mLinkProviders; to
QSet<LinkProvider*> m_linkProviders;

while in KdeConnectPluginPrivate you changed
Device* mDevice; to
Device* device;

Also, I see you renamed d-pointers (https://wiki.qt.io/D-Pointer) like QScopedPointer<KdeConnectPluginPrivate> d; to QScopedPointer<KdeConnectPluginPrivate> m_pluginInfos; and we should keep the naming so it is clear we are using this programming pattern.

This revision now requires changes to proceed.Aug 16 2017, 1:20 PM

In D7312#136345, @albertvaka wrote:
I see some inconsistencies, for example in DaemonPrivate you changed
QSet<LinkProvider*> mLinkProviders; to
`QSet<LinkProvider*> m_linkProviders;`
while in KdeConnectPluginPrivate you changed
`Device* mDevice;` to
`Device* device; `
Also, I see you renamed d-pointers (https://wiki.qt.io/D-Pointer) like QScopedPointer<KdeConnectPluginPrivate> d; to QScopedPointer<KdeConnectPluginPrivate> m_pluginInfos; and we should keep the naming so it is clear we are using this programming pattern.

I did not know about d-pointers, that's my bad :). I'll revert the names and make all member variables inside the structures coherent with the rest.

What do you think about naming them d_ptr like in the Qt doc? It would be more explicit to newcomers that this is deliberate.

In D7312#136390, @jeanv wrote:

What do you think about naming them d_ptr like in the Qt doc? It would be more explicit to newcomers that this is deliberate.

In most KDE projects they are just called 'd', and I think that adding '_ptr' won't make it that more clear, but do as as you wish :)

In D7312#136722, @albertvaka wrote:

In D7312#136390, @jeanv wrote:

What do you think about naming them d_ptr like in the Qt doc? It would be more explicit to newcomers that this is deliberate.

In most KDE projects they are just called 'd', and I think that adding '_ptr' won't make it that more clear, but do as as you wish :)

No problem, I'll leave it as it is.

Reverted the erroneous renaming of d-pointers. And properly renamed a few missing member variables

So, is the revision good?

In D7312#142075, @jeanv wrote:

So, is the revision good?

Yes, sorry, I will push it now.

albertvaka accepted this revision as: KDE Connect.Sep 3 2017, 7:42 PM

albertvaka accepted this revision.

This revision is now accepted and ready to land.Sep 3 2017, 7:42 PM

Closed by commit R224:71a4c5b6d9bf: Make member variable names, & placement and * placement more coherent (authored by jeanv, committed by albertvaka). · Explain WhySep 3 2017, 7:45 PM

This revision was automatically updated to reflect the committed changes.

nicolasfella mentioned this in D7209: Fix Notifications in Plasmoid.Oct 13 2017, 7:36 PM

Large Diff

This large diff affects 159 files. Files without inline comments have been collapsed. Expand All Files

Revision Contents
Changeset List

			Path	Packages
M			app/main.cpp (2 lines)
M			app/org.kde.kdeconnect.app.desktop (3 lines)
M			cli/kdeconnect-cli.cpp (2 lines)
M			core/backends/bluetooth/bluetoothdownloadjob.h (2 lines)
M			core/backends/bluetooth/bluetoothdownloadjob.cpp (2 lines)
M			core/backends/bluetooth/bluetoothuploadjob.h (2 lines)
M			core/backends/bluetooth/bluetoothuploadjob.cpp (2 lines)
M			core/backends/devicelinereader.h (12 lines)
M			core/backends/devicelinereader.cpp (16 lines)
M			core/backends/devicelink.h (16 lines)
M			core/backends/devicelink.cpp (12 lines)
M			core/backends/lan/downloadjob.h (10 lines)
M			core/backends/lan/downloadjob.cpp (30 lines)
M			core/backends/lan/landevicelink.h (6 lines)
M			core/backends/lan/landevicelink.cpp (38 lines)
M			core/backends/lan/lanlinkprovider.h (22 lines)
M			core/backends/lan/lanlinkprovider.cpp (130 lines)
M			core/backends/lan/server.cpp (2 lines)
M			core/backends/lan/socketlinereader.h (16 lines)
M			core/backends/lan/socketlinereader.cpp (14 lines)
M			core/backends/lan/uploadjob.h (12 lines)
M			core/backends/lan/uploadjob.cpp (72 lines)
M			core/backends/pairinghandler.cpp (2 lines)
M			core/core_debug.cpp (4 lines)
M			core/daemon.h (16 lines)
M			core/daemon.cpp (60 lines)
M			core/device.h (4 lines)
M			core/device.cpp (12 lines)
M			core/filetransferjob.h (20 lines)
M			core/filetransferjob.cpp (64 lines)
M			core/kdeconnectconfig.h (10 lines)
M			core/kdeconnectconfig.cpp (88 lines)
M			core/kdeconnectplugin.h (2 lines)
M			core/kdeconnectplugin.cpp (36 lines)
M			core/kdeconnectpluginconfig.h (2 lines)
M			core/kdeconnectpluginconfig.cpp (50 lines)
M			core/networkpackage.h (54 lines)
M			core/networkpackage.cpp (40 lines)
M			core/pluginloader.h (2 lines)
M			core/pluginloader.cpp (2 lines)
M			daemon/kdeconnect.desktop (2 lines)
M			daemon/kdeconnectd.desktop.cmake (1 line)
M			fileitemactionplugin/kdeconnectsendfile.desktop (2 lines)
M			fileitemactionplugin/sendfileitemaction.h (2 lines)
M			fileitemactionplugin/sendfileitemaction.cpp (4 lines)
M			indicator/deviceindicator.h (2 lines)
M			indicator/main.cpp (6 lines)
M			indicator/org.kde.kdeconnect.nonplasma.desktop (2 lines)
M			interfaces/dbusinterfaces.h (10 lines)
M			interfaces/dbusinterfaces.cpp (2 lines)
M			interfaces/devicesmodel.h (4 lines)
M			interfaces/devicesmodel.cpp (6 lines)
M			interfaces/devicessortproxymodel.h (2 lines)
M			interfaces/devicessortproxymodel.cpp (2 lines)
M			interfaces/modeltest.h (10 lines)
M			interfaces/modeltest.cpp (4 lines)
M			interfaces/notificationsmodel.cpp (2 lines)
M			kcm/kcm.h (2 lines)
M			kcm/kcm.cpp (4 lines)
M			kcm/kcm_kdeconnect.desktop (3 lines)
M			kcm/org.kde.kdeconnect.kcm.desktop (2 lines)
M			kcmplugin/kdeconnectpluginkcm.h (2 lines)
M			kcmplugin/kdeconnectpluginkcm.cpp (18 lines)
M			kio/kiokdeconnect.h (12 lines)
M			kio/kiokdeconnect.cpp (12 lines)
M			plasmoid/declarativeplugin/kdeconnectdeclarativeplugin.h (2 lines)
M			plasmoid/declarativeplugin/kdeconnectdeclarativeplugin.cpp (16 lines)
M			plasmoid/declarativeplugin/objectfactory.h (4 lines)
M			plasmoid/declarativeplugin/processrunner.h (2 lines)
M			plasmoid/declarativeplugin/processrunner.cpp (2 lines)
M			plasmoid/declarativeplugin/responsewaiter.h (4 lines)
M			plasmoid/package/metadata.desktop (2 lines)
M			plugins/battery/batterydbusinterface.h (10 lines)
M			plugins/battery/batterydbusinterface.cpp (14 lines)
M			plugins/battery/batteryplugin.h (2 lines)
M			plugins/battery/batteryplugin.cpp (2 lines)
M			plugins/battery/kdeconnect_battery.json (2 lines)
M			plugins/clipboard/clipboardplugin.h (2 lines)
M			plugins/clipboard/clipboardplugin.cpp (2 lines)
M			plugins/clipboard/kdeconnect_clipboard.json (2 lines)
M			plugins/findmyphone/findmyphoneplugin.h (2 lines)
M			plugins/findmyphone/kdeconnect_findmyphone.json (2 lines)
M			plugins/kdeconnect.notifyrc (18 lines)
M			plugins/kdeconnect_plugin.desktop (1 line)
M			plugins/lockdevice/kdeconnect_lockdevice.json (2 lines)
M			plugins/lockdevice/lockdeviceplugin.h (2 lines)
M			plugins/mousepad/kdeconnect_mousepad.json (2 lines)
M			plugins/mousepad/mousepadplugin.h (4 lines)
M			plugins/mousepad/mousepadplugin.cpp (10 lines)
M			plugins/mousepad_windows/kdeconnect_mousepad.json (2 lines)
M			plugins/mousepad_windows/mousepadplugin_windows.h (2 lines)
M			plugins/mpriscontrol/kdeconnect_mpriscontrol.json (2 lines)
M			plugins/mpriscontrol/mpriscontrolplugin.h (2 lines)
M			plugins/mprisremote/kdeconnect_mprisremote.json (1 line)
M			plugins/mprisremote/mprisremoteplugin.h (2 lines)
M			plugins/notifications/kdeconnect_notifications.json (2 lines)
M			plugins/notifications/notification.h (55 lines)
M			plugins/notifications/notification.cpp (105 lines)
M			plugins/notifications/notificationsdbusinterface.h (16 lines)
M			plugins/notifications/notificationsdbusinterface.cpp (73 lines)
M			plugins/notifications/notificationsplugin.h (2 lines)
M			plugins/notifications/sendreplydialog.h (6 lines)
M			plugins/notifications/sendreplydialog.cpp (8 lines)
M			plugins/pausemusic/kdeconnect_pausemusic.json (2 lines)
M			plugins/pausemusic/kdeconnect_pausemusic_config.desktop (1 line)
M			plugins/pausemusic/pausemusic_config.h (2 lines)
M			plugins/pausemusic/pausemusic_config.cpp (2 lines)
M			plugins/pausemusic/pausemusicplugin.h (2 lines)
M			plugins/ping/kdeconnect_ping.json (1 line)
M			plugins/ping/pingplugin.h (2 lines)
M			plugins/remotecommands/kdeconnect_remotecommands.json (2 lines)
M			plugins/remotecommands/remotecommandsplugin.h (6 lines)
M			plugins/remotecommands/remotecommandsplugin.cpp (4 lines)
M			plugins/remotecontrol/kdeconnect_remotecontrol.json (2 lines)
M			plugins/remotecontrol/remotecontrolplugin.h (2 lines)
M			plugins/remotecontrol/remotecontrolplugin.cpp (2 lines)
M			plugins/remotekeyboard/kdeconnect_remotekeyboard.json (2 lines)
M			plugins/remotekeyboard/remotekeyboardplugin.h (2 lines)
M			plugins/runcommand/kdeconnect_runcommand.json (2 lines)
M			plugins/runcommand/kdeconnect_runcommand_config.desktop (1 line)
M			plugins/runcommand/runcommand_config.h (6 lines)
M			plugins/runcommand/runcommand_config.cpp (14 lines)
M			plugins/runcommand/runcommandplugin.h (2 lines)
M			plugins/screensaver-inhibit/kdeconnect_screensaver_inhibit.json (2 lines)
M			plugins/screensaver-inhibit/screensaverinhibitplugin.h (2 lines)
M			plugins/sendnotifications/kdeconnect_sendnotifications.json (2 lines)
M			plugins/sendnotifications/kdeconnect_sendnotifications_config.desktop (1 line)
M			plugins/sendnotifications/notificationslistener.h (6 lines)
M			plugins/sendnotifications/notificationslistener.cpp (54 lines)
M			plugins/sendnotifications/notifyingapplication.h (6 lines)
M			plugins/sendnotifications/notifyingapplication.cpp (4 lines)
M			plugins/sendnotifications/notifyingapplicationmodel.h (4 lines)
M			plugins/sendnotifications/notifyingapplicationmodel.cpp (4 lines)
M			plugins/sendnotifications/sendnotifications_config.h (2 lines)
M			plugins/sendnotifications/sendnotifications_config.cpp (2 lines)
M			plugins/sendnotifications/sendnotificationsplugin.h (2 lines)
M			plugins/sftp/kdeconnect_sftp.json (2 lines)
M			plugins/sftp/mounter.h (2 lines)
M			plugins/sftp/sftpplugin.h (4 lines)
M			plugins/sftp/sftpplugin.cpp (38 lines)
M			plugins/share/kdeconnect_share.json (2 lines)
M			plugins/share/kdeconnect_share_config.desktop (1 line)
M			plugins/share/share_config.h (2 lines)
M			plugins/share/share_config.cpp (2 lines)
M			plugins/share/shareplugin.h (2 lines)
M			plugins/telephony/kdeconnect_telephony.json (2 lines)
M			plugins/telephony/sendsmsdialog.h (6 lines)
M			plugins/telephony/sendsmsdialog.cpp (8 lines)
M			plugins/telephony/telephonyplugin.h (2 lines)
M			plugins/telephony/telephonyplugin.cpp (2 lines)
M			tests/downloadjobtest.cpp (26 lines)
M			tests/lanlinkprovidertest.cpp (108 lines)
M			tests/networkpackagetests.cpp (2 lines)
M			tests/pluginloadtest.cpp (10 lines)
M			tests/sendfiletest.cpp (10 lines)
M			tests/testnotificationlistener.cpp (12 lines)
M			tests/testsocketlinereader.cpp (66 lines)
M			tests/testsslsocketlinereader.cpp (178 lines)
M			urlhandler/org.kde.kdeconnect.telhandler.desktop (1 line)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	18151			Aug 14 2017, 4:37 PM	★	★
Diff 2	18167			Aug 15 2017, 8:47 AM	★	★
Diff 3	18304			Aug 17 2017, 6:54 PM	★	★
Diff 4	19147	5d4821f	R224:71a4c5b6d9bf70ef6d76126c7753b9bee9d5eca0	Sep 3 2017, 7:42 PM	★	★

Make member variable names, & placement and * placement more coherentClosedPublicActions

Details

Diff Detail

Large Diff

Revision ContentsChangeset List

Diff 18304

app/main.cpp

app/org.kde.kdeconnect.app.desktop

cli/kdeconnect-cli.cpp

core/backends/bluetooth/bluetoothdownloadjob.h

core/backends/bluetooth/bluetoothdownloadjob.cpp

core/backends/bluetooth/bluetoothuploadjob.h

core/backends/bluetooth/bluetoothuploadjob.cpp

core/backends/devicelinereader.h

core/backends/devicelinereader.cpp

core/backends/devicelink.h

core/backends/devicelink.cpp

core/backends/lan/downloadjob.h

core/backends/lan/downloadjob.cpp

core/backends/lan/landevicelink.h

core/backends/lan/landevicelink.cpp

core/backends/lan/lanlinkprovider.h

core/backends/lan/lanlinkprovider.cpp

core/backends/lan/server.cpp

core/backends/lan/socketlinereader.h

core/backends/lan/socketlinereader.cpp

core/backends/lan/uploadjob.h

core/backends/lan/uploadjob.cpp

core/backends/pairinghandler.cpp

core/core_debug.cpp

core/daemon.h

core/daemon.cpp

core/device.h

core/device.cpp

core/filetransferjob.h

core/filetransferjob.cpp

core/kdeconnectconfig.h

core/kdeconnectconfig.cpp

core/kdeconnectplugin.h

core/kdeconnectplugin.cpp

core/kdeconnectpluginconfig.h

core/kdeconnectpluginconfig.cpp

core/networkpackage.h

core/networkpackage.cpp

core/pluginloader.h

core/pluginloader.cpp

daemon/kdeconnect.desktop

daemon/kdeconnectd.desktop.cmake

fileitemactionplugin/kdeconnectsendfile.desktop

fileitemactionplugin/sendfileitemaction.h

fileitemactionplugin/sendfileitemaction.cpp

indicator/deviceindicator.h

indicator/main.cpp

indicator/org.kde.kdeconnect.nonplasma.desktop

interfaces/dbusinterfaces.h

interfaces/dbusinterfaces.cpp

interfaces/devicesmodel.h

interfaces/devicesmodel.cpp

interfaces/devicessortproxymodel.h

interfaces/devicessortproxymodel.cpp

interfaces/modeltest.h

interfaces/modeltest.cpp

interfaces/notificationsmodel.cpp

kcm/kcm.h

kcm/kcm.cpp

kcm/kcm_kdeconnect.desktop

kcm/org.kde.kdeconnect.kcm.desktop

kcmplugin/kdeconnectpluginkcm.h

kcmplugin/kdeconnectpluginkcm.cpp

kio/kiokdeconnect.h

kio/kiokdeconnect.cpp

plasmoid/declarativeplugin/kdeconnectdeclarativeplugin.h

plasmoid/declarativeplugin/kdeconnectdeclarativeplugin.cpp

plasmoid/declarativeplugin/objectfactory.h

plasmoid/declarativeplugin/processrunner.h

plasmoid/declarativeplugin/processrunner.cpp

plasmoid/declarativeplugin/responsewaiter.h

plasmoid/package/metadata.desktop

plugins/battery/batterydbusinterface.h

plugins/battery/batterydbusinterface.cpp

Make member variable names, & placement and * placement more coherent
ClosedPublic
Actions

Revision Contents
Changeset List