D24821: Add ManagedConfigModule

2019-10-22 Thread Kevin Ottens
This revision was automatically updated to reflect the committed changes. Closed by commit R296:c8b95b07c53b: Add ManagedConfigModule (authored by ervin). REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24821?vs=68530&id=68545 REVISION DETAIL https://pha

D24821: Add ManagedConfigModule

2019-10-22 Thread David Edmundson
davidedmundson accepted this revision. REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D24821 To: ervin, #plasma, #frameworks, mart, bport, davidedmundson Cc: davidedmundson, bport, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24821: Add ManagedConfigModule

2019-10-22 Thread Kevin Ottens
ervin updated this revision to Diff 68530. ervin added a comment. Address David's comments REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24821?vs=68411&id=68530 REVISION DETAIL https://phabricator.kde.org/D24821 AFFECTED FILES src/quickaddons/CM

D24821: Add ManagedConfigModule

2019-10-22 Thread Kevin Ottens
ervin marked an inline comment as done. REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D24821 To: ervin, #plasma, #frameworks, mart, bport, davidedmundson Cc: davidedmundson, bport, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24821: Add ManagedConfigModule

2019-10-22 Thread Kevin Ottens
ervin marked 4 inline comments as done. REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D24821 To: ervin, #plasma, #frameworks, mart, bport, davidedmundson Cc: davidedmundson, bport, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24821: Add ManagedConfigModule

2019-10-21 Thread Nathaniel Graham
ngraham added a dependent revision: D24822: Port the desktoptheme kcm to ManagedConfigModule. REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D24821 To: ervin, #plasma, #frameworks, mart, bport, davidedmundson Cc: davidedmundson, bport, kde-frameworks-devel, LeGast0

D24821: Add ManagedConfigModule

2019-10-21 Thread David Edmundson
davidedmundson accepted this revision. davidedmundson added a comment. > Indeed the alternative is registerSettings(), I went for the less explicit "children settings" mechanism mostly because it was closer to the older system around KCModule which was very much working by convention. Ha

D24821: Add ManagedConfigModule

2019-10-21 Thread Kevin Ottens
ervin added a comment. In D24821#551179 , @davidedmundson wrote: > Nice! > > I'm not super sold on magically doing findChildren to get the config skeletons over an explicit > registerSettings(KCoreConfigSkeleton*). > > I'm not against

D24821: Add ManagedConfigModule

2019-10-21 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > managedconfigmodule.h:116 > + * > + * If you want to make the ConfigModule available only conditionally (i.e. > show in > + * the list of available modules only if some test succeeds) then you can use I know this is just copy paste, but ca

D24821: Add ManagedConfigModule

2019-10-21 Thread Benjamin Port
bport accepted this revision. bport added a comment. This revision is now accepted and ready to land. Ok for me, when David remarks are addressed REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D24821 To: ervin, #plasma, #frameworks, mart, bport Cc: davidedmundso

D24821: Add ManagedConfigModule

2019-10-21 Thread David Edmundson
davidedmundson added a comment. Nice! I'm not super sold on magically doing findChildren to get the config skeletons over an explicit registerSettings(KCoreConfigSkeleton*). I'm not against it either, but could you expand on the rationale. INLINE COMMENTS > managedconfigmodule.cp

D24821: Add ManagedConfigModule

2019-10-21 Thread Kevin Ottens
ervin updated this revision to Diff 68411. ervin added a comment. Addresses bport's comment REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24821?vs=68408&id=68411 REVISION DETAIL https://phabricator.kde.org/D24821 AFFECTED FILES src/quickaddons/C

D24821: Add ManagedConfigModule

2019-10-21 Thread Benjamin Port
bport requested changes to this revision. bport added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > managedconfigmodule.cpp:96 > +{ > +return false; > +} Like above I think we need to set it to true > managedconfigmodule.h:210 > + * > + * By defau

D24821: Add ManagedConfigModule

2019-10-21 Thread Kevin Ottens
ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY This is a new type of ConfigModule which will manage the state of "apply"