D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-24 Thread Méven Car 
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:07b4f3fb6c3c: Solid-device-automounter/kcm: Use KConfigXT 
in ui (authored by meven).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27127?vs=76286=76289

REVISION DETAIL
  https://phabricator.kde.org/D27127

AFFECTED FILES
  solid-device-automounter/kcm/DeviceAutomounterKCM.cpp
  solid-device-automounter/kcm/DeviceAutomounterKCM.h
  solid-device-automounter/kcm/DeviceAutomounterKCM.ui

To: meven, ervin, ngraham, #plasma, bport, crossi
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

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-24 Thread Méven Car 
meven updated this revision to Diff 76286.
meven marked 3 inline comments as done.
meven added a comment.


  Fix indentation and add intermediate variable

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27127?vs=75891=76286

BRANCH
  arcpatch-D27127_1

REVISION DETAIL
  https://phabricator.kde.org/D27127

AFFECTED FILES
  solid-device-automounter/kcm/DeviceAutomounterKCM.cpp
  solid-device-automounter/kcm/DeviceAutomounterKCM.h
  solid-device-automounter/kcm/DeviceAutomounterKCM.ui

To: meven, ervin, ngraham, #plasma, bport, crossi
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

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-24 Thread Kevin Ottens 
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))
>  {

This is wrongly indented

> ervin wrote in DeviceAutomounterKCM.cpp:114
> I'd store the result of the automountEnabled call in an intermediate 
> variable, just to make things more readable.

Comment has been marked done but I don't see it changed here. Wrong status of 
the review?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
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

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Méven Car 
meven marked 2 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> meven wrote in DeviceAutomounterKCM.cpp:57
> Just waiting for this review then, I am preparing the next patch

D27480 

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Méven Car 
meven marked 4 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> ervin wrote in DeviceAutomounterKCM.cpp:57
> Sure, moving away from a singleton is always intrusive (just like moving away 
> from a global variable which it is really). Let's aim for it in a different 
> specific patch.

Just waiting for this review then, I am preparing the next patch

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Kevin Ottens 
ervin added inline comments.

INLINE COMMENTS

> meven wrote in DeviceAutomounterKCM.cpp:57
> Please beware this will make my patch quite a lot more intrusive, DeviceModel 
> for instance, will need a field to keep some reference to the 
> AutomounterSettings

Sure, moving away from a singleton is always intrusive (just like moving away 
from a global variable which it is really). Let's aim for it in a different 
specific patch.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> ervin wrote in DeviceAutomounterKCM.cpp:57
> I tend to consider this as a step back to be honest. Singletons tend to be 
> more trouble down the line when something goes wrong.

Please beware this will make my patch quite a lot more intrusive, DeviceModel 
for instance, will need a field to keep some reference to the 
AutomounterSettings

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Kevin Ottens 
ervin added inline comments.

INLINE COMMENTS

> meven wrote in DeviceAutomounterKCM.cpp:57
> I removed the m_settings instead, relying solely on the singleton.

I tend to consider this as a step back to be honest. Singletons tend to be more 
trouble down the line when something goes wrong.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> bport wrote in DeviceAutomounterKCM.cpp:84
> We can also change kcfgc to add parent in constructor

It would imply make AutomounterSettings not-static.
I would need to change the DeviceModel that uses AutomounterSettings static 
functions as well.

> ervin wrote in DeviceAutomounterKCM.cpp:57
> Missing this as parent, this is leaked. Also I'd likely move that in the ctor 
> initialization list (same thing for m_devices actually, maybe in another 
> patch, your call).

I removed the m_settings instead, relying solely on the singleton.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-18 Thread Méven Car 
meven updated this revision to Diff 75891.
meven marked an inline comment as done.
meven added a comment.


  Remove m_settings AutomounterSettings member from DeviceAutomounterKCM

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27127?vs=75845=75891

BRANCH
  arcpatch-D27127

REVISION DETAIL
  https://phabricator.kde.org/D27127

AFFECTED FILES
  solid-device-automounter/kcm/DeviceAutomounterKCM.cpp
  solid-device-automounter/kcm/DeviceAutomounterKCM.h
  solid-device-automounter/kcm/DeviceAutomounterKCM.ui

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-17 Thread Benjamin Port 
bport added inline comments.

INLINE COMMENTS

> DeviceAutomounterKCM.cpp:84
>  saveLayout();
> +delete m_settings;
>  }

We can also change kcfgc to add parent in constructor

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-17 Thread Méven Car 
meven updated this revision to Diff 75845.
meven marked 2 inline comments as done.
meven added a comment.


  Add a delete for m_settings, move m_devices to ctor intialization

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27127?vs=74925=75845

BRANCH
  arcpatch-D27127

REVISION DETAIL
  https://phabricator.kde.org/D27127

AFFECTED FILES
  solid-device-automounter/kcm/DeviceAutomounterKCM.cpp
  solid-device-automounter/kcm/DeviceAutomounterKCM.h
  solid-device-automounter/kcm/DeviceAutomounterKCM.ui

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-12 Thread Kevin Ottens 
ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> DeviceAutomounterKCM.cpp:57
>  
> +m_settings = new AutomounterSettings();
> +addConfig(m_settings, this);

Missing this as parent, this is leaked. Also I'd likely move that in the ctor 
initialization list (same thing for m_devices actually, maybe in another patch, 
your call).

> DeviceAutomounterKCM.cpp:114
> +
> +kcfg_AutomountUnknownDevices->setEnabled(m_settings->automountEnabled());
> +kcfg_AutomountOnLogin->setEnabled(m_settings->automountEnabled());

I'd store the result of the automountEnabled call in an intermediate variable, 
just to make things more readable.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27127

To: meven, ervin, ngraham, #plasma, bport, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

2020-02-03 Thread Méven Car 
meven created this revision.
meven added reviewers: ervin, ngraham, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
meven requested review of this revision.

REVISION SUMMARY
  - Make default button work
  - Move connection from cpp to .ui
  - Slight label change

TEST PLAN
  Use default button

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27127

AFFECTED FILES
  solid-device-automounter/kcm/DeviceAutomounterKCM.cpp
  solid-device-automounter/kcm/DeviceAutomounterKCM.h
  solid-device-automounter/kcm/DeviceAutomounterKCM.ui

To: meven, ervin, ngraham, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart