- User Since
- Apr 18 2015, 11:52 AM (221 w, 2 d)
update to first review feedback
Thanks for first review & testing.
Default is false, so does not change behaviour unless someone toggles the switch, right?
Closing due to lack of further interest by anyone for long time
Sat, Jul 13
Fri, Jul 12
Reasons why currently I favour this over D16882:
- slightly less complex, as it does more what one expects, adding & removing actions on show & hide
- no global static
While trying to understand the problem, I found that perhaps we could still revert to using the aboutToHide signal, with proper boundaroes.
So alternative solution proposal up as D22424
Thu, Jul 11
Hm, seeing all this hardcoded code for optional plugins, this calls out for someone to look into making this something pulled in from the language plugins instead :) Not your fault, just mentioning the obvious. So should be fine to just add here.
Wed, Jul 10
Tue, Jul 9
Mon, Jul 8
No time for Plasma in the next months reserved, so withdrawing as reviewer/commenter to clear my todo list
Ping? Please keep hammering the iron while hot :)
@rjvbb Hi, please Abandon this review, as the reply has been that this should be solved on KDE Frameworks level, in KAboutData (as well as with respective new ECM module for getting the VCS/git revision info into the buildsystem).
Will close next week given no-one has cared for > 2 years.
Will close next week given no-one has cared for 2 years and the affected code also changed.
@rjvbb Hi, please use the "Abandon this Revision" action, given that this patch/feature was not accepted, so the list of patches to review is not cluttered for everyone.
@tristanp Hi. You do not have push rights for KDE code reposistories, correct? So someone else would need to push for you then. Okay to extend your author name in the commit message to "Tristan Porteries"?
Sun, Jul 7
Sat, Jul 6
@cullmann @dhaumann Hi, please pardon my direct pushing of this and the one before, this are emergency fixes for issues only found the last hour and due to tagging being done today/tomorrow, and I am pretty sure about these fixes.
Please post-push review :) Future changes should come as normal review requests again.
@mlaurent Hi, sorry to default to you here, but given the risks with foreach porting (I have done my own share of mistakes in such despite all concentration :) ) https://bugs.kde.org/show_bug.cgi?id=408801 might be triggered by one of this here, have not yet investigated more.
With you mind perhaps still with memories about the code from the foreach porting work, could you given that bug a look where this might be due to containers being modified in a range-for loop?
Fri, Jul 5
Thanks for your work, log now as wanted, thus closing as resolved
Interesting approach, might work out, thanks for doing this :)
FTR now that all dep builds are updated, the plasma-frameworks part of the Plasma product also all passed, so all as it should.
This runs the chance to break some 3rd-party software which also calls the Q_DECLARE_METATYPE(KPluginMetaData) macro.
Any chance this could be moved to the place that needs this for now, and perhaps add a KF6 TODO instead?
Hi. Seems CI is not happy about something here: https://build.kde.org/view/Failing/job/Plasma/job/plasma-framework/ please have a look.
Thu, Jul 4
No maintainer here, also no opinion :) Thus resigning to not keep wrong hopes for feedback from me.
Hi. I am sorry, somehow missed this review request in the long list on phabricator before. Change looks good to me. Can you push this change yourself, or shall I? What author credentials would you like with the commit (name & email address)?
Currently no time for Marble, so resigning.