D25746: Adapt to change in KConfigCompiler

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. My gut feeling would be to keep the item and the property names in sync unlike now... but this is a good fix as well indeed. REPOSITORY R296 KDeclarative BRANCH master REVISION DETAIL

D25520: GridViewKCM expose a property to disable the GridView in a KCM without disabling the whole KCM

ervin added a comment. In D25520#567651 , @broulik wrote: > Alternatively we could fix the scroll view container taking into account the `scroll` enabled, so that `view.enabled` does the right thing You mean the other way around right? Li

D25520: GridViewKCM expose a property to disable the GridView in a KCM without disabling the whole KCM

ervin added a comment. In D25520#567644 , @davidedmundson wrote: > Throwing out an alternative suggestion > > readonly property alias gridContainer: scroll > > > (and then your code calling gridContainer.enabled = whatever) > > I

D25520: GridViewKCM expose a property to disable the GridView in a KCM without disabling the whole KCM

ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D25520 To: crossi, #plasma, ervin, bport, mart, davidedmundson Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25149: Add a new template for KCMs

ervin added inline comments. INLINE COMMENTS > kcm.cpp:37 > +KAboutData* about = new KAboutData( > +QStringLiteral("kcm_%{APPNAMELC}"), i18n("%{APPNAME} Configuration > Module"), > +QStringLiteral("0.1"), QString(), KAboutLicense::GPL, Nitpick: indentation looks broken here, I think

D25149: Add a new template for KCMs

ervin added inline comments. INLINE COMMENTS > tcanabrava wrote in kcm.cpp:43 > What's the correct form then? No need to connect the settings *at all*? No need indeed... it's *magic*! ;-) > tcanabrava wrote in main.qml:39 > The default fake setting will not be immutable, or you mean that you wa

D25149: Add a new template for KCMs

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > %{APPNAMELC}settings.kcfgc:5 > +DefaultValueGetters=true > +GenerateProperties=true You also want "ParentInConstructor=true" in here. > kcm.cpp:32 > +: KQu

D25286: Allow to disable autosave behavior in ConfigPropertyMap

This revision was automatically updated to reflect the committed changes. Closed by commit R296:40b94af6f0f4: Allow to disable autosave behavior in ConfigPropertyMap (authored by ervin). REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25286?vs=69676&id=696

D25286: Allow to disable autosave behavior in ConfigPropertyMap

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, davidedmundson, bport. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY This is especially important when ConfigPropertyMap

D25210: Prepare KConfigSkeletonItem to allow inheriting its private class

This revision was automatically updated to reflect the committed changes. Closed by commit R237:44cfa0631d25: Prepare KConfigSkeletonItem to allow inheriting its private class (authored by ervin). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25210?vs=69440&i

D25211: Add KPropertySkeletonItem

This revision was automatically updated to reflect the committed changes. Closed by commit R237:d63955cfe547: Add KPropertySkeletonItem (authored by ervin). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25211?vs=69628&id=69674 REVISION DETAIL https://phabr

D25211: Add KPropertySkeletonItem

ervin updated this revision to Diff 69628. ervin added a comment. Avoid potential ownership problem on the propertyName REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25211?vs=69442&id=69628 REVISION DETAIL https://phabricator.kde.org/D25211 AFFECTED FI

D25249: KDirModel: port to qCDebug, with its own category

ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D25249 To: dfaure, mlaurent, sandsmark, ervin Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25211: Add KPropertySkeletonItem

ervin updated this revision to Diff 69442. ervin added a comment. Fix member variable naming convention REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25211?vs=69432&id=69442 REVISION DETAIL https://phabricator.kde.org/D25211 AFFECTED FILES src/core/k

D25210: Prepare KConfigSkeletonItem to allow inheriting its private class

ervin added a comment. In D25210#559977 , @vkrause wrote: > Ok as such, but I think KConfigSkeletonItemPrivate would need a virtual dtor for this to actually work safely when used by a sub-class? You mean like not leaking memory? woops. ;

D25210: Prepare KConfigSkeletonItem to allow inheriting its private class

ervin updated this revision to Diff 69440. ervin added a comment. Add missing virtual dtor REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25210?vs=69431&id=69440 REVISION DETAIL https://phabricator.kde.org/D25210 AFFECTED FILES src/core/kcoreconfigske

D25209: Register KKeySequenceWidget to KConfigDialogManager

This revision was automatically updated to reflect the committed changes. Closed by commit R265:c0a8cd4a2a95: Register KKeySequenceWidget to KConfigDialogManager (authored by ervin). REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25209?vs=69430&id=69439

D25208: Add missing property to KKeySequenceWidget

This revision was automatically updated to reflect the committed changes. Closed by commit R263:fab6d95a9fad: Add missing property to KKeySequenceWidget (authored by ervin). REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25208?vs=69429&id=69436 REVISION DETAI

D25210: Prepare KConfigSkeletonItem to allow inheriting its private class

ervin created this revision. ervin added reviewers: Frameworks, dfaure, davidedmundson, bport, crossi. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.

D25211: Add KPropertySkeletonItem

ervin created this revision. ervin added reviewers: Frameworks, dfaure, davidedmundson, bport, crossi. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY This new item allows to use a QObject property as

D25209: Register KKeySequenceWidget to KConfigDialogManager

ervin created this revision. ervin added reviewers: Frameworks, dfaure, davidedmundson, bport. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY This makes it usable in a KConfigXT context. Depends

D25208: Add missing property to KKeySequenceWidget

ervin created this revision. ervin added reviewers: Frameworks, dfaure, davidedmundson, bport. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY Oddly enough we had all the necessary to manage keySequenc

D25149: Add a new Template for KCM's.

ervin added a comment. What about having the template use kcfg and ManagedConfigModule as well? It'd give better behaving modules by default and is a better practice than starting without them. I'd rather have people make the conscious decision to remove them because it turns out it's a KCM

D25072: Disable the restore defaults button if the KCModule says so

This revision was automatically updated to reflect the committed changes. Closed by commit R295:f2f04d67e221: Disable the restore defaults button if the KCModule says so (authored by ervin). REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25072?vs=69042&id=692

D25070: Make KCModuleQml conform to the defaulted() signal

This revision was automatically updated to reflect the committed changes. Closed by commit R295:4dc999b9eba1: Make KCModuleQml conform to the defaulted() signal (authored by ervin). REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25070?vs=69040&id=69272 REVIS

D25071: Have KCModuleProxy take care of the defaulted state

This revision was automatically updated to reflect the committed changes. Closed by commit R295:dc836403bb5d: Have KCModuleProxy take care of the defaulted state (authored by ervin). REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25071?vs=69270&id=69273 REVI

D25069: Adjust KCModule to also channel information about defaults

This revision was automatically updated to reflect the committed changes. Closed by commit R265:0762203eb9df: Adjust KCModule to also channel information about defaults (authored by ervin). REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25069?vs=69269&i

D25071: Have KCModuleProxy take care of the defaulted state

ervin updated this revision to Diff 69270. ervin added a comment. Add missing @since markers REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25071?vs=69041&id=69270 REVISION DETAIL https://phabricator.kde.org/D25071 AFFECTED FILES src/kcmoduleproxy.cp

D25069: Adjust KCModule to also channel information about defaults

ervin updated this revision to Diff 69269. ervin added a comment. Add missing @since markers REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25069?vs=69039&id=69269 REVISION DETAIL https://phabricator.kde.org/D25069 AFFECTED FILES src/kcmodule.c

D25070: Make KCModuleQml conform to the defaulted() signal

ervin added inline comments. INLINE COMMENTS > davidedmundson wrote in kcmoduleqml.cpp:80-82 > needsSave emits the current state and then connects for changes > representsDefaults only connects for changes > > I would expect them to match as they're doing equivalent things. > > It looks to me

D25071: Have KCModuleProxy take care of the defaulted state

ervin added a comment. In D25071#556681 , @davidedmundson wrote: > Edit: ah, I see why. > > KCMMultiDialog and system settings re-evaluate the buttons on receipt of the changed signal > Still seems maybe a bit odd, but it makes sense in c

D25077: Use compile time checked connect

This revision was automatically updated to reflect the committed changes. Closed by commit R296:92f6e85438f8: Use compile time checked connect (authored by ervin). REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25077?vs=69051&id=69052 REVISION DETAIL h

D25077: Use compile time checked connect

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY Benjamin made me realized we went through introspection for

D25071: Have KCModuleProxy take care of the defaulted state

ervin added inline comments. INLINE COMMENTS > davidedmundson wrote in kcmoduleproxy.cpp:196-197 > Why emit changed? > > Given KCModuleProxy mirrors KCModule API I would have expected a defaulted > signal to match? Because all the consumers I found surprisingly don't care about the distinction

D25069: Adjust KCModule to also channel information about defaults

ervin marked an inline comment as done. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25069 To: ervin, #frameworks, #plasma, mart, davidedmundson, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25073: Get KQuickAddons::ConfigModule to expose if we're in the defaults state

This revision was automatically updated to reflect the committed changes. Closed by commit R296:b2788c1a0894: Get KQuickAddons::ConfigModule to expose if we're in the defaults state (authored by ervin). REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25073

D25075: Make the settingChanged() slot protected.

This revision was automatically updated to reflect the committed changes. Closed by commit R296:372dbb493df6: Make the settingChanged() slot protected. (authored by ervin). REPOSITORY R296 KDeclarative CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25075?vs=69045&id=69048 REVISION D

D25075: Make the settingChanged() slot protected.

ervin created this revision. ervin added reviewers: Plasma, Frameworks, bport, mart, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY This might be necessary for some subclasses which ha

D25073: Get KQuickAddons::ConfigModule to expose if we're in the defaults state

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY ManagedConfigModule is adjusted as well to manage that new

D25070: Make KCModuleQml conform to the defaulted() signal

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, davidedmundson, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY This new signal got introduced in KCModule, so we n

D25072: Disable the restore defaults button if the KCModule says so

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, davidedmundson, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.

D25071: Have KCModuleProxy take care of the defaulted state

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, davidedmundson, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY Now that KCModule exposes a defaulted() signal, con

D25069: Adjust KCModule to also channel information about defaults

ervin created this revision. ervin added reviewers: Frameworks, Plasma, mart, davidedmundson, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY We'd probably do things differently nowadays (inclu

D25061: kconfig_compiler: Move the KSharedConfig::Ptr when using them

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. LGTM, maybe give some times to others to chip in. REPOSITORY R237 KConfig BRANCH master REVISION DETAIL https://phabricator.kde.org/D25061 To: aacid, ervin Cc: kde-frameworks-devel, e

D24821: Add ManagedConfigModule

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

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

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

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

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

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

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"

D24716: Fix memory leak of KQuickAddons::ConfigModule objects

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. Looks good to me, I'll accept it but better wait a bit in case others want to chip in. REPOSITORY R295 KCMUtils BRANCH master REVISION DETAIL https://phabricator.kde.org/D24716 To: d

D24716: Fix memory leak of KQuickAddons::ConfigModule objects

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcmoduleloader.cpp:101 > } else { > KQuickAddons::ConfigModule *cm = > factory->create(nullptr, args2); > if (!cm

D24697: Expose isImmutable to introspection (e.g. QML)

This revision was automatically updated to reflect the committed changes. Closed by commit R237:3f68be12f87e: Expose isImmutable to introspection (e.g. QML) (authored by ervin). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24697?vs=68046&id=68052 REVISION D

D24697: Expose isImmutable to introspection (e.g. QML)

ervin created this revision. ervin added reviewers: Plasma, Frameworks, mart, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D24697 AFFEC

D24490: Make kconfig_compiler generate ctors with the optional parent arg

This revision was automatically updated to reflect the committed changes. Closed by commit R237:5fa1a3312ab2: Make kconfig_compiler generate ctors with the optional parent arg (authored by ervin). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24490?vs=67516&i

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

This revision was automatically updated to reflect the committed changes. Closed by commit R237:4c3d37519684: Add convenience for defaults/dirty states to KCoreConfigSkeleton (authored by ervin). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24494?vs=67597&id

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin added a comment. Ah I see what you meant now. I'd rather not do this, it's IMO better if we remove the setting completely when KF6 comes. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D24490 To: ervin, #plasma, #frameworks, dfaure, mart, apol Cc: kossebau, ap

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

ervin updated this revision to Diff 67597. ervin added a comment. Add the KF6 comment REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24494?vs=67522&id=67597 REVISION DETAIL https://phabricator.kde.org/D24494 AFFECTED FILES autotests/kconfigskeletontes

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin added a comment. In D24490#544599 , @apol wrote: > It could make sense to tweak it so in KF6 `ParentInConstructor=true` it's true by default. What do you mean by tweak? REPOSITORY R237 KConfig REVISION DETAIL https://phabricat

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin added a comment. Any chance to get another round of reviews now that this patch changed quite a bit? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D24490 To: ervin, #plasma, #frameworks, dfaure, mart, apol Cc: kossebau, apol, kde-frameworks-devel, LeGast00n,

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

ervin added a comment. Any chance to get another round of reviews now that this patch changed quite a bit? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D24494 To: ervin, #plasma, #frameworks, dfaure, mart Cc: kossebau, davidedmundson, kde-frameworks-devel, LeGast0

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

ervin added inline comments. INLINE COMMENTS > davidedmundson wrote in kcoreconfigskeleton.cpp:140 > Do we need to make this > > if (d->mIsDefaultImpl){ > return d->mIsDefaultImpl(); > } > return false; > > and initialize mIsDefaultImpl to nullptr > > so that it doesn't crash if som

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

ervin updated this revision to Diff 67522. ervin added a comment. 5.63 -> 5.64 REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24494?vs=67507&id=67522 REVISION DETAIL https://phabricator.kde.org/D24494 AFFECTED FILES autotests/kconfigskeletontest.cpp

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin updated this revision to Diff 67516. ervin added a comment. Address Friedrich's comments, I went for hiding the feature behind a flag after all, it was just less complexity added to the compiler in the end REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.or

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin added a comment. In D24490#543810 , @kossebau wrote: > In D24490#543807 , @ervin wrote: > > > In D24490#543724 , @kossebau wrote: > > > > > How do

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin added a comment. In D24490#543724 , @kossebau wrote: > How does this effect the BIC of the generated classes? Consumers might have exposed the generated classes in public API. Any chance this could become a opt-in change for KF5 times?

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

ervin updated this revision to Diff 67507. ervin added a comment. Second try, without breaking ABI... it's the best workaround I found in the current situation, I admit I died a bit inside. REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24494?vs=67496&id=6

D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

ervin created this revision. ervin added reviewers: Plasma, Frameworks, dfaure, mart. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REVISION SUMMARY It allows to verify if all the items of the skeleton are in their

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin added inline comments. INLINE COMMENTS > apol wrote in test_notifiers.cpp.ref:7 > Maybe it would be better to use the parent argument in KConfigSkeleton? > explicit KConfigSkeleton(const QString &configname = QString(), QObject > *parent = nullptr); It's what I attempted first, the proble

D24490: Make kconfig_compiler generate ctors with the optional parent arg

ervin created this revision. ervin added reviewers: Plasma, Frameworks, dfaure, mart. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ervin requested review of this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D24490 AFFEC

Re: CI system maintainability

code style, etc.). Regards. -- Kevin Ottens, http://ervin.ipsquad.net signature.asc Description: This is a digitally signed message part.

Re: CI system maintainability

Hello, On Friday, 29 March 2019 09:43:44 CET Volker Krause wrote: > On Friday, 29 March 2019 08:59:59 CET Kevin Ottens wrote: > > On Thursday, 28 March 2019 21:53:06 CET Alexander Neundorf wrote: > > > Having mandatory reviews for a central and complex component like > &g

Re: CI system maintainability

responsibility in carrying those duties for the well being of the project. > IMO this specific case is also a technical issue. Akonadi makes things > complicated (and it's already 13 years old, so it should be mature in the > meantime). Yes, it's byzantine really. And the user experience is still not great (to be polite), had to setup some new hardware recently and I was honestly almost to the point of throwing it all through the window. Regards. -- Kevin Ottens, http://ervin.ipsquad.net signature.asc Description: This is a digitally signed message part.

Re: CI system maintainability

t regarding the state of the CI in PIM components. Although it's clear from the kde-pim list that the CI is making a lot of noise there. Either we collectively became too good at ignoring those emails, or it's too verbose (after all it's one email per component per build type, it piles up quickly). Regards. -- Kevin Ottens, http://ervin.ipsquad.net signature.asc Description: This is a digitally signed message part.

Re: CI system maintainability

friend told me when I started > to work on kde "We can't create a bug when we don't code..." And this is true, writing code will create bugs, that's why responsible people like safety nets even to the price of throughput. Now can we use a more adult tone again? Maybe re-read Dan's email again who has a more balanced view despite being in a similar situation? Regards. -- Kevin Ottens, http://ervin.ipsquad.net signature.asc Description: This is a digitally signed message part.

Re: CI system maintainability

programmer :-) I think this is > especially important for projects like PIM, where most of us contribute at > work in between waiting for CI and replying to 15 Slack threads or in the > evening after a long day And this too of course. I fully support this message. ;-) Cheers. -- Kev

Re: CI system maintainability

Hello, On Thursday, 28 March 2019 10:35:37 CET Luca Beltrame wrote: > In data giovedì 28 marzo 2019 10:32:39 CET, Kevin Ottens ha scritto: > > OK, to be fair not 100% today's situation because of the above. It was > > based on best judgment maybe we're missing su

Re: CI system maintainability

Hello, On Thursday, 28 March 2019 10:08:54 CET Luca Beltrame wrote: > In data giovedì 28 marzo 2019 09:50:47 CET, Kevin Ottens ha scritto: > > I'd argue we're loosing more with the current state of PIM than we'd loose > > with mandatory reviews. > > Perhaps, i

Re: CI system maintainability

Hello, On Thursday, 28 March 2019 09:41:29 CET Luca Beltrame wrote: > In data giovedì 28 marzo 2019 09:29:22 CET, Kevin Ottens ha scritto: > > at your screen or pair with you" in the past. Clearly this compromise gets > > somewhat exploited and that's especially bad in

Re: CI system maintainability

mandatory... I know it's unpopular in KDE, and I advocated for "don't force a tool if you can get someone to look at your screen or pair with you" in the past. Clearly this compromise gets somewhat exploited and that's especially bad in the case of a fragile and central

D9015: Refactoring the hidding/showing animation use within KFilePlacesView

ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9015 To: franckarrecot, ervin, renatoo, mlaurent, ngraham Cc: #frameworks

D9015: Refactoring the hidding/showing animation use within KFilePlacesView

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplacesview.cpp:811-813 > +if (result != nullptr) { > > +if (result == emptyTrash) { This whole change on the if structure there is unrelated

D8450: User can now hide an entire places group from KFilePlacesView

ervin accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8450 To: franckarrecot, ngraham, renatoo, ervin, mwolff, mlaurent Cc: mwolff, #frameworks

D8367: Hidding place groups implementation in KFilePlacesModel

ervin accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8367 To: franckarrecot, renatoo, ngraham, ervin, mwolff, mlaurent Cc: mwolff, ngraham, mlaurent, #frameworks

D8450: User can now hide an entire places group from KFilePlacesView

ervin added inline comments. INLINE COMMENTS > franckarrecot wrote in kfileplacesview.cpp:863-866 > Oki I add that to my list :-) Cool, I'll wait for that extra review to appear before accepting that one. So that we don't forget it. :-) REPOSITORY R241 KIO REVISION DETAIL https://phabrica

D8367: Hidding place groups implementation in KFilePlacesModel

ervin added inline comments. INLINE COMMENTS > franckarrecot wrote in kfileplacesmodel.cpp:870 > Because it wouldn't compile without the whole KFIlePlacesModel:: prefix, so I > end up going for variable renaming, seemed cleaner Let's go for naming it "groupHidden" then. It's a weird shortening

D8450: User can now hide an entire places group from KFilePlacesView

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ervin wrote in kfileplacesview.cpp:863-866 > This duplicates code from the next if branch below... I wonder if before or > after that commit we shouldn't try to

D8367: Hidding place groups implementation in KFilePlacesModel

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. A small nitpick otherwise lgtm. INLINE COMMENTS > kfileplacesmodel.cpp:870 > > -bookmark.setMetaDataItem(QStringLiteral("IsHidden"), (hidden ? > QStringLiteral("true") : QStr

D8332: Added baloo urls into places model

ervin accepted this revision. ervin added inline comments. INLINE COMMENTS > renatoo wrote in kfileplacesmodel.cpp:68 > This code came from dolphin. And it does not check at runtime for changes on > the configuration. > > But yes I agree keep track of this will be nice, I thought about that whi

D8566: Add API to retrieve the screen id for a screen name

ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D8566 To: amantia, #plasma, ervin, dvratil, mlaurent, hein, davidedmundson Cc: broulik, plasma-devel, dhauman

D8450: User can now hide an entire places group from KFilePlacesView

ervin accepted this revision. This revision is now accepted and ready to land. REVISION DETAIL https://phabricator.kde.org/D8450 To: mlaurent, ngraham, renatoo, franckarrecot, ervin Cc: #frameworks

D8367: Hidding place groups implementation in KFilePlacesModel

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Couple more changes needed. INLINE COMMENTS > kfileplacesmodel.cpp:79-83 > +//static const QHash > s_groupTypeToStateName > +//{ > +//return *s_groupTypeToStateName; > +//} > +

D8366: Factoring out lists of url data within KFilePlacesModelTest

ervin accepted this revision. This revision is now accepted and ready to land. REVISION DETAIL https://phabricator.kde.org/D8366 To: mlaurent, renatoo, ervin, franckarrecot Cc: ervin, ngraham, mlaurent, #frameworks

D8566: Add API to retrieve the screen id for a screen name

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > corona.h:223 > + * @returns the id of the screen for a specified name > + * -1 is returned there is no such screen. > + * @since 5.40 "if there is n

D8367: Hidding place groups implementation in KFilePlacesModel

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > mlaurent wrote in kfileplacesitem.cpp:202 > This one is for avoiding duplicate code as we create in this patch isHidden > method. That was kind of my point, is

D8450: User can now hide an entire places group from KFilePlacesView

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. At least the unit test would be welcome, I let you decide on the other comment. INLINE COMMENTS > kfileplacesmodel.cpp:329 > > +QModelIndexList KFilePlacesModel::groupIndexes(con

D8348: Add a section for removable devices

ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8348 To: renatoo, #dolphin, #frameworks, #vdg, ervin Cc: mlaurent, anthonyfieroni, ngraham, #frameworks

D8367: Hidding place groups implementation in KFilePlacesModel

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplacesmodeltest.cpp:504 > QList args; > -QSignalSpy spy_inserted(m_places, > SIGNAL(rowsInserted(QModelIndex,int,int))); > -QSignalSpy spy_rem

D8366: Factoring out lists of url data within KFilePlacesModelTest

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplacesmodeltest.cpp:177 > > +static const QStringList initalListOfPlaces() > +{ Typo: initial > kfileplacesmodeltest.cpp:182 > + > +static const QString

<    1   2   3   4   5   6   7   8   9   10   >