Re: RFC: Running clang-format across all Plasma (and more?) repos

Am 2019-07-11 16:18, schrieb David Edmundson: One topic discussed at the recent Plasma sprint was that we should run a code formatting tool (clang-format) over all our repos to ease all future review comments about whitespace. All new contributions simply have to run the same tool and we get con

D22333: Move Solid::Device::listFromQuery calls to a separate thread

apol added a comment. In D22333#494389 , @bruns wrote: > Again, where is it blocking? Which backend? udisks2 mainly, but every backend can block by its virtue. > listFromQuery can be replaced by an asynchronous "enumerate(predicate)"

D22333: Move Solid::Device::listFromQuery calls to a separate thread

bruns added a comment. In D22333#494384 , @apol wrote: > In D22333#494253 , @bruns wrote: > > > In D22333#494152 , @apol wrote: > > > > > In D22333#49414

D22333: Move Solid::Device::listFromQuery calls to a separate thread

apol added a comment. In D22333#494253 , @bruns wrote: > In D22333#494152 , @apol wrote: > > > In D22333#494146 , @bruns wrote: > > > > > Why not just a

D22401: change debug dir order to prefer appDir and do not duplicate Debuggers

apol added inline comments. INLINE COMMENTS > debugger.cpp:145 > +// as binary blob (e.g. Windows exe). > + > QString(QStringLiteral("%1/%2")).arg(QCoreApplication::applicationDirPath(), > path), > +// Search in default path The QString constructor isn't necessary. Actua

Re: RFC: Running clang-format across all Plasma (and more?) repos

Hi, On 7/11/19 5:18 PM, David Edmundson wrote: One topic discussed at the recent Plasma sprint was that we should run a code formatting tool (clang-format) over all our repos to ease all future review comments about whitespace. This is a good idea! Reviewing such changes can become really pain

Re: RFC: Running clang-format across all Plasma (and more?) repos

On Thu, Jul 11, 2019 at 3:32 PM alcinos wrote: > > Hi, > > Just to mention that we have a .clang-format in Kdenlive as well > (https://invent.kde.org/kde/kdenlive/blob/master/.clang-format) Oh awesome, do you mind if I ask some questions? Did running it across the base cause any issues? Did it

D22403: qobject_cast rather than dynamic_cast

sitter added inline comments. INLINE COMMENTS > broulik wrote in reportassistantdialog.cpp:285 > Why does it check for `page` here but not down below? (Unrelated question to > this patch) the entire class is a hot mess WRT pointer management I think REPOSITORY R871 DrKonqi BRANCH qobjectc

D22404: do not dereference `current` outside guard condition

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY if current can be null, then clearly the one if must be nested inside the guard if lest it trips ov

D22403: qobject_cast rather than dynamic_cast

broulik accepted this revision. broulik added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > reportassistantdialog.cpp:285 > +ReportAssistantPage *page = qobject_cast *>(currentPage()->widget()); > +if (page && !page->showNextPage()) { > +retur

D22403: qobject_cast rather than dynamic_cast

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY doesn't require RTTI and is faster (I am not sure why there are casts at all instead of pointer

D22402: only benchmark once

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY benchmarking N times slows down the test considerably for what feels like zero testing advantage. g

D22333: Move Solid::Device::listFromQuery calls to a separate thread

bruns added a comment. In D22333#494152 , @apol wrote: > In D22333#494146 , @bruns wrote: > > > Why not just a singleshot timer from the constructor? Avoids any initial blocking ... > > > Initi

RFC: Running clang-format across all Plasma (and more?) repos

One topic discussed at the recent Plasma sprint was that we should run a code formatting tool (clang-format) over all our repos to ease all future review comments about whitespace. All new contributions simply have to run the same tool and we get consistent code without having to comment on every

D22397: [Fonts KCM] Alter DPI only on explicit user interaction

This revision was automatically updated to reflect the committed changes. Closed by commit R119:e9a38fbd4d6a: [Fonts KCM] Alter DPI only on explicit user interaction (authored by broulik). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22397?vs=61591&id

D22191: Implement syncing of theme preferences between SDDM and Plasma

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. In D22191#493985 , @filipf wrote: > If that's the case then the whole KCM needs to be disabled because any change made within it entails wri

D22385: [mobile/wifi] Port settings to Kirigami Formlayout

This revision was automatically updated to reflect the committed changes. Closed by commit R116:d6625e528410: [mobile/wifi] Port settings to Kirigami Formlayout (authored by nicolasfella). REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/

D22385: [mobile/wifi] Port settings to Kirigami Formlayout

nicolasfella added a comment. Alright, thank you Jan! REPOSITORY R116 Plasma Network Management Applet BRANCH form REVISION DETAIL https://phabricator.kde.org/D22385 To: nicolasfella, jgrulich Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot

D22401: change debug dir order to prefer appDir and do not duplicate Debuggers

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY previously we'd duplicate 'codeNames' of debuggers. so, if I had `CodeName=gdb` in both `bin/deb

D22400: disambiguate the names of Debugger

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY name() is in fact the localized name so rename it to displayName() this isn't clear when reading

D22399: prevent exhausting the maximum size of bug reports

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY bugzilla has a hardcoded server-side limit for how large a given comment may be. this can somewhat

D22397: [Fonts KCM] Alter DPI only on explicit user interaction

fvogt added a comment. > Haven't checked whether this is in 5.16 or only a master regression. 5.16.3 appears to have the same bug here. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22397 To: broulik, #plasma Cc: fvogt, plasma-devel, LeGast00n, jraleigh

D22398: always log raw exception data

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY a UI representation of the error may lack all the data. specifically the json blobs could be fairly

D22397: [Fonts KCM] Alter DPI only on explicit user interaction

broulik created this revision. broulik added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY The settings are loaded after the QML is created, and once it is loaded, it becomes indeterminist

D22381: Add previous-/nextActivity methods

muesli updated this revision to Diff 61590. muesli added a comment. Move nameBaseOrdering to an anonymous namespace and mark it as inline. REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22381?vs=61589&id=61590 BRANCH prevnext-activity (

D22381: Add previous-/nextActivity methods

muesli updated this revision to Diff 61589. muesli added a comment. Addressed @ivan's change requests Summary: - Renamed sorting method to nameBaseOrdering - Remove activity from sorted cache without re-sorting the entire list - Switch to prev/next running activity, skipping other

D22381: Add previous-/nextActivity methods

muesli updated this revision to Diff 61588. muesli added a comment. Use a QVector instead of a QList to store sorted activities. REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22381?vs=61579&id=61588 BRANCH prevnext-activity (branched f

D22381: Add previous-/nextActivity methods

ivan requested changes to this revision. ivan added a comment. This revision now requires changes to proceed. Inserting/Removing/Updating a sorted list does not need to resort every time - removing is easy, adding a new item is std::lower_bound (a binary search), and updating is a combination

KDE CI: Plasma » drkonqi » kf5-qt5 WindowsMSVCQt5.11 - Build # 61 - Still Failing!

BUILD FAILURE Build URL https://build.kde.org/job/Plasma/job/drkonqi/job/kf5-qt5%20WindowsMSVCQt5.11/61/ Project: kf5-qt5 WindowsMSVCQt5.11 Date of build: Thu, 11 Jul 2019 12:07:17 + Build duration: 1 min 45 sec and counting CONSOLE OUTPUT [...tr

D22393: remove UnhandledErrorDialog

This revision was automatically updated to reflect the committed changes. Closed by commit R871:a23fe3a38a30: remove UnhandledErrorDialog (authored by sitter). REPOSITORY R871 DrKonqi CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22393?vs=61574&id=61587 REVISION DETAIL https://ph

D22396: Fix typo in CREATE_GETTER_AND_SETTER

This revision was automatically updated to reflect the committed changes. Closed by commit R161:d15446776e23: Fix typo in CREATE_GETTER_AND_SETTER (authored by muesli). REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22396?vs=61581&id=61586 R

D22333: Move Solid::Device::listFromQuery calls to a separate thread

apol added a comment. In D22333#494146 , @bruns wrote: > Why not just a singleshot timer from the constructor? Avoids any initial blocking ... Initial, but doesn't fix the problem. We could potentially delay this few seconds instead, but

D22333: Move Solid::Device::listFromQuery calls to a separate thread

bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Why not just a singleshot timer from the constructor? Avoids any initial blocking ... REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22333 To: a

D22394: Polish IPv4 settings dialog

This revision was automatically updated to reflect the committed changes. Closed by commit R116:8d5b0b7f140c: Polish IPv4 settings dialog (authored by apol). REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22394?vs=61575&id=61584 REVIS

D22396: Fix typo in CREATE_GETTER_AND_SETTER

muesli created this revision. muesli added a reviewer: ivan. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. muesli requested review of this revision. REVISION SUMMARY Fix typo when undefining CREATE_GETTER_AND_SETTER. REPOSITORY R161 KActivity Manager Service BRANCH

D22381: Add previous-/nextActivity methods

muesli updated this revision to Diff 61579. muesli added a comment. Keep an alphabetically sorted list of activities Summary: Instead of re-sorting the activity list every time previous-/nextActivity gets called, maintain a sorted list. REPOSITORY R161 KActivity Manager Service CHA

D22034: Introduce ContainmentLayoutManager QML plugin

mart updated this revision to Diff 61576. mart added a comment. - allow a limited auto expanding for applets - click on empty areas always closes edit mode - wire up maximum size - start on a new logic for default sizes - event comppress sizehints - take minimum size into account -

D22035: Port FolderView to ContainmentLayoutManager plugin

mart updated this revision to Diff 61577. mart added a comment. - adapt to api change - add migration script REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22035?vs=60493&id=61577 BRANCH mart/newlayout REVISION DETAIL https://phabricator.kde.

D22394: Polish IPv4 settings dialog

apol created this revision. apol added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. Herald added a reviewer: jgrulich. apol requested review of this revision. REVISION SUMMARY Set the hostname as the placeholder text, which will be used if the host

D22393: remove UnhandledErrorDialog

sitter created this revision. sitter added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY the dialog was actually broken through at least all of plasma5 :/ this is no longer necessary. i

D22344: Expose some more settings in an Advanced dialog

broulik added inline comments. INLINE COMMENTS > ipv4widget.cpp:451 > +auto dadTimeout = new QSpinBox; > +dadTimeout->setMinimum(-1); > +dadTimeout->setValue(m_tmpIpv4Setting.dadTimeout()); a `specialValueText()` for -1 "Default" would have been lovely > ipv4widget.cpp:460 > + > +

D22344: Expose some more settings in an Advanced dialog

This revision was automatically updated to reflect the committed changes. Closed by commit R116:1e001b46838e: Expose some more settings in an Advanced dialog (authored by apol). REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22344?vs=6

D22322: Store crash report automatically if shutting down

sitter accepted this revision. sitter added a comment. some style fixes then ship it plz 👍 INLINE COMMENTS > drkonqi.cpp:214 > + > +void removeOldFilesIn(QDir& dir) { > +auto fileList = dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot, & goes to the right of the space; curly

D7820: man ioslave: spurious numbers included in clang(1) man page

marten added a comment. Confirmed that man:clang(1) now correctly displays the man page with no spurious numbers shown. Would be happy to abandon this review request. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D7820 To: marten, #plasma, kfm-devel, mkoller Cc