ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> DeviceAutomounterKCM.cpp:44
> : KCModule(parent, args)//DeviceAutomounterKCMFactory::componentData(),
> parent)
> - , m_devices(new DeviceModel(this))
> + , m_settings(new AutomounterSettings(this)), m_devices(new
> DeviceModel(this, m_settings))
> {
please break the line before ", m_devices"
> DeviceModel.cpp:34
> +DeviceModel::DeviceModel(QObject *parent, AutomounterSettings *m_settings)
> + : QAbstractItemModel(parent), m_settings(m_settings)
> {
m_settings on its own line?
> DeviceModel.h:37
> public:
> - explicit DeviceModel(QObject *parent = nullptr);
> + explicit DeviceModel(QObject *parent, AutomounterSettings *m_settings);
> ~DeviceModel() override = default;
parent should come last and keep the = nullptr
> DeviceAutomounter.cpp:111
> Solid::Device dev(udi);
> - automountDevice(dev, AutomounterSettings::Attach);
> - AutomounterSettings::self()->save();
> + automountDevice(dev, m_settings->Attach);
> + m_settings->save();
This is rather odd. A too eager search and replace? ;-)
> AutomounterSettings.cpp:23
>
> +AutomounterSettings::AutomounterSettings(QObject *parent) :
> + AutomounterSettingsBase(parent)
I guess you could have used `using
AutomounterSettingsBase::AutomounterSettingsBase;` in the header instead.
> AutomounterSettings.h:31
> public:
> + AutomounterSettings( QObject *parent = nullptr );
> enum AutomountType {
No space between the parenthesis
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27480
To: meven, ervin, crossi, bport, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
