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

[Martin Flöser]

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
consistent code without having to comment on every minor thing in a
review individually.

I've written up a wall of text outlining steps, challenges etc.
https://phabricator.kde.org/T11214

Does anyone have any thoughts / objections?


As someone being guilty of only focusing on whitespace changes I'm very 
much in favor of this idea.


Cheers
Martin

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

[Aleix Pol Gonzalez]

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)" 
call which uses the existing DeviceAdded signal. This would also remove the 
inherent race between the listFromQuery and DeviceAdded/DeviceRemoved.
  
  I've looked into changes that could be done to solid to improve this, nothing 
felt like a good step forward. This approach works, makes sense and shows 
results.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Stefan Brüns]

bruns added a comment.


  In D22333#494384 , @apol wrote:
  
  > In D22333#494253 , @bruns wrote:
  >
  > > In D22333#494152 , @apol wrote:
  > >
  > > > 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 we'd still be getting an odd ~500ms freeze randomly
  > > >  Or we can just decide to move blocking or heavy algorithms to separate 
threads and enjoy our multicore computers and a fluid experience.
  > >
  > >
  > > Where does the blocking happen? How do you guarantee none of the later 
call block? Large parts of the code are executed in the main thread anyway, 
only the initial list creation happens in a worker thread.
  >
  >
  > listFromQuery is the big blocking call we are moving into a separate thread.
  
  
  Again, where is it blocking? Which backend?
  
  >> Also, if this is a generic problem, why only fix it in the dataengines, 
not in Solid itself?
  > 
  > This would mean refactoring how Solid works and even coming up with 
entirely new concepts. I don't discard doing it at some point but I don't think 
plasmashell startup blockage is reason enough to redo a framework that has been 
working for about 5 years as is.
  
  listFromQuery can be replaced by an asynchronous "enumerate(predicate)" call 
which uses the existing DeviceAdded signal. This would also remove the inherent 
race between the listFromQuery and DeviceAdded/DeviceRemoved.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Aleix Pol Gonzalez]

apol added a comment.


  In D22333#494253 , @bruns wrote:
  
  > In D22333#494152 , @apol wrote:
  >
  > > 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 we'd still be getting an odd ~500ms freeze randomly
  > >  Or we can just decide to move blocking or heavy algorithms to separate 
threads and enjoy our multicore computers and a fluid experience.
  >
  >
  > Where does the blocking happen? How do you guarantee none of the later call 
block? Large parts of the code are executed in the main thread anyway, only the 
initial list creation happens in a worker thread.
  
  
  listFromQuery is the big blocking call we are moving into a separate thread.
  
  > Also, if this is a generic problem, why only fix it in the dataengines, not 
in Solid itself?
  
  This would mean refactoring how Solid works and even coming up with entirely 
new concepts. I don't discard doing it at some point but I don't think 
plasmashell startup blockage is reason enough to redo a framework that has been 
working for about 5 years as is.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Aleix Pol Gonzalez]

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.
Actually it would be better written as `QCoreApplication::applicationDirPath() 
+ QLatin1Char('/') + path`

> debugger.cpp:151
> +QHash result;
> +for (const auto &debuggerDir: debuggerDirs) {
>  QStringList debuggers = QDir(debuggerDir).entryList(QDir::Files);

qAsConst(debuggerDirs);

> debugger.cpp:152
> +for (const auto &debuggerDir: debuggerDirs) {
>  QStringList debuggers = QDir(debuggerDir).entryList(QDir::Files);
> +for (const auto &debuggerFile : debuggers) {

const

REPOSITORY
  R871 DrKonqi

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

To: sitter, #plasma
Cc: apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, mart

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

[Vlad Zagorodniy]

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 painful 
for contributors as well developers. This will also save us time 
disputing coding style, e.g. "I prefer aligning pointers to left in this 
case, change my mind."



I've written up a wall of text outlining steps, challenges etc.
https://phabricator.kde.org/T11214


Given that we follow the kdelibs/Frameworks coding style in KWin, I 
don't see any reason to not run clang-format. See HACKING.md.


It is worth to mention that clang-format doesn't fix completely all 
coding style issues, e.g. one of those issues is braces. As you know, we 
have to put braces even if the body of a conditional statement contains 
only one line. clang-format doesn't have any style option to enforce 
this rule. Luckily, clang-tidy can do that [1].


So, perhaps we need to run both clang-format as well clang-tidy for some 
certain projects, e.g. KWin.


Cheers,
Vlad

[1] 
https://clang.llvm.org/extra/clang-tidy/checks/readability-braces-around-statements.html

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

[David Edmundson]

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 help save time in future commits / reviews?
Is the lack of enforcement proving problematic?

David

D22403: qobject_cast rather than dynamic_cast

[Harald Sitter]

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
  qobjectcast

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

To: sitter, #plasma, broulik
Cc: broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22404: do not dereference `current` outside guard condition

[Harald Sitter]

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 over the nullptr when calling `name()`.

TEST PLAN
  builds

REPOSITORY
  R871 DrKonqi

BRANCH
  save-deref

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

AFFECTED FILES
  src/bugzillaintegration/reportassistantdialog.cpp

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22403: qobject_cast rather than dynamic_cast

[Kai Uwe Broulik]

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()) {
> +return;

Why does it check for `page` here but not down below? (Unrelated question to 
this patch)

REPOSITORY
  R871 DrKonqi

BRANCH
  qobjectcast

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

To: sitter, #plasma, broulik
Cc: broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22403: qobject_cast rather than dynamic_cast

[Harald Sitter]

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 members...)

TEST PLAN
  builds

REPOSITORY
  R871 DrKonqi

BRANCH
  qobjectcast

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

AFFECTED FILES
  src/bugzillaintegration/reportassistantdialog.cpp

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22402: only benchmark once

[Harald Sitter]

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. getting just some rough benchmark data should
  be entirely sufficient for a unit test. when more detailed
  benchmarking is required one can easily switch the benchmark macro

TEST PLAN
  6 times faster test pass

REPOSITORY
  R871 DrKonqi

BRANCH
  faster-test

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

AFFECTED FILES
  src/tests/backtraceparsertest/backtraceparsertest.cpp

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

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

[Stefan Brüns]

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 ...
  >
  >
  > Initial, but doesn't fix the problem. We could potentially delay this few 
seconds instead, but we'd still be getting an odd ~500ms freeze randomly
  >  Or we can just decide to move blocking or heavy algorithms to separate 
threads and enjoy our multicore computers and a fluid experience.
  
  
  Where does the blocking happen? How do you guarantee none of the later call 
block? Large parts of the code are executed in the main thread anyway, only the 
initial list creation happens in a worker thread.
  
  Also, if this is a generic problem, why only fix it in the dataengines, not 
in Solid itself?

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[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
consistent code without having to comment on every minor thing in a
review individually.

I've written up a wall of text outlining steps, challenges etc.
https://phabricator.kde.org/T11214

Does anyone have any thoughts / objections?

David

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

[Kai Uwe Broulik]

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=61597

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

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

To: broulik, #plasma, ngraham, bshah
Cc: fvogt, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22191: Implement syncing of theme preferences between SDDM and Plasma

[Nathaniel Graham]

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 writing to root. This would be relevant to the 
function though when abstracted and used throughout KCMs. Another thing that 
crosses my mind in regards to failing is BSD (due to different generic paths), 
but I have no idea if they even use SDDM.
  
  
  Hmm, that's true, so this is a pre-existing issue then.
  
  Okay, let's get this in and focus on polishing it in subsequent revisions!

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  sddm-theme-syncing (branched from master)

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

To: filipf, #plasma, ngraham, davidedmundson, #vdg
Cc: leinir, cfeck, GB_2, ndavis, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Nicolas Fella]

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?vs=61532&id=61596

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

AFFECTED FILES
  mobile/wifi/package/contents/ui/IPAddressSetting.qml
  mobile/wifi/package/contents/ui/NetworkSettings.qml
  mobile/wifi/package/contents/ui/WirelessSecuritySetting.qml

To: nicolasfella, jgrulich
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Nicolas Fella]

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, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Harald Sitter]

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/debugger/internal/gdbrc` and
  also `XDG_*/debugger/internal/gdbrc` it'd effectively include both in the
  list. and it was more or less undefined which one would get used.
  with the revised lookup code we'll now always have unique CodeNames (e.g.
  gdb will only appear once in the candidate list) and by preferring bin/
  we can now put "fake" debuggers into $builddir/bin/ to force them getting
  used over potentially system-wide debuggers. the latter is particularly
  handy when testing since you can now fixate the gdb debugger to ultimately
  be nothing more than `cat /sometracefile` but rely on all the same code
  paths as an actual gdb run would

TEST PLAN
  builds and I can dump fake debuggers into my build dir

REPOSITORY
  R871 DrKonqi

BRANCH
  debugger-load-override

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

AFFECTED FILES
  src/debugger.cpp

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22400: disambiguate the names of Debugger

[Harald Sitter]

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 the code

TEST PLAN
  builds

REPOSITORY
  R871 DrKonqi

BRANCH
  debugger-name

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

AFFECTED FILES
  src/backtracewidget.cpp
  src/debugger.cpp
  src/debugger.h
  src/debuggerlaunchers.cpp
  src/drkonqibackends.cpp

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22399: prevent exhausting the maximum size of bug reports

[Harald Sitter]

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 easily get triggered by heavily threaded
  applications as the backtrace would include a lot of "noise" threads.
  when this previously happened drkonqi would find itself incapable of
  filing the report and leaving the user to their own devices.
  
  now we are much smarter about this
  if the reportinterface finds that the bug description is too long it will
  try to use a reduced backtrace (only a couple of lines from the relevant
  thread) or when that too is too long to exclude the backtrace entirely.
  in both events it would then additionally attach the complete backtrace
  to the bug.
  
  for this there's a bunch of rejiggering necessary:
  
  - generateReportFullText has had its signature changed, instead of using a 
lazy bool now use enums to control the output. this should be vastly more 
readable
  - new Backtrace control on generateReportFullText ranging Complete> 
Redcued>Exclude
  - addedToCC was renamed to attachBacktraceWithReport which is a more suitable 
name for what it does. it's logic has been moved to a new attachBacktrace which 
requires the caller to set the actual comment
  - attachBacktrace is called with a full report comment when used via \ 
attachBacktraceWithReport (i.e. the user indicated that their crash is in fact 
the same as another report, so their report gets attached to the other one). 
it's called with a simplified message when attaching as part of a 
reduced/excluded backtrace dance
  
  the end result is that the reports can no longer exhaust the hardcoded
  character limit.
  
  there is one caveat: the user can still write a super long description
  manually and exhaust the limit. this is however very hard to do in
  real life without "abusing" the description for something.
  
  CHANGELOG: Bug reporting can no longer exhaust the maximum character size 
enforced by Bugzilla
  BUG: 248807
  FIXED-IN: 5.17.0

TEST PLAN
  - file bug report with ETOOLONGTRACE -> gets reduced and attached
  - regular attaching still works
  - exhausting the limit *exactly* works as expected

REPOSITORY
  R871 DrKonqi

BRANCH
  trace-exclude

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

AFFECTED FILES
  src/bugzillaintegration/reportassistantdialog.cpp
  src/bugzillaintegration/reportassistantpages_base.cpp
  src/bugzillaintegration/reportassistantpages_bugzilla.cpp
  src/bugzillaintegration/reportinterface.cpp
  src/bugzillaintegration/reportinterface.h

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

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

[Fabian Vogt]

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, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22398: always log raw exception data

[Harald Sitter]

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 long if perl hits a server-side exception
  (e.g. bug in bugzilla itself). by logging it we have a way to get to
  the raw data should it be necessary

TEST PLAN
  builds and warns when appropriate

REPOSITORY
  R871 DrKonqi

BRANCH
  exception-warn

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

AFFECTED FILES
  src/bugzillaintegration/libbugzilla/exceptions.cpp

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

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

[Kai Uwe Broulik]

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 indeterministic which property (checkbox checked or spinbox value) is 
re-evaluated first and the `Binding` ends up resetting the config.

TEST PLAN
  Opened Fonts KCM, DPI 192 was properly shown, changed some other font 
settings, my DPI did not change
  Changed font DPI, worked fine. Checking and unchecking also works fine. When 
unchecking it resets the value to 96

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

To: broulik, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22381: Add previous-/nextActivity methods

[Christian Muehlhaeuser]

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 (branched from master)

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

AFFECTED FILES
  src/common/dbus/org.kde.ActivityManager.Activities.xml
  src/service/Activities.cpp
  src/service/Activities.h
  src/service/Activities_p.h

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22381: Add previous-/nextActivity methods

[Christian Muehlhaeuser]

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 states

REPOSITORY
  R161 KActivity Manager Service

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22381?vs=61588&id=61589

BRANCH
  prevnext-activity (branched from master)

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

AFFECTED FILES
  src/common/dbus/org.kde.ActivityManager.Activities.xml
  src/service/Activities.cpp
  src/service/Activities.h
  src/service/Activities_p.h

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22381: Add previous-/nextActivity methods

[Christian Muehlhaeuser]

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 from master)

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

AFFECTED FILES
  src/common/dbus/org.kde.ActivityManager.Activities.xml
  src/service/Activities.cpp
  src/service/Activities.h
  src/service/Activities_p.h

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22381: Add previous-/nextActivity methods

[Ivan Čukić]

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 of the two.

INLINE COMMENTS

> Activities.cpp:54
>  
> +static
> +bool infoLessThan(const ActivityInfo &info, const ActivityInfo &other)

You can use anonymous namespace for this (instead of `static`) or just make it 
a non-static function.

It can be marked as `inline`, although the compiler will probably do that 
regardless of you saying so.

You can rename it to something like `nameBasedOrdering` - better communicates 
what it does.

> Activities.cpp:349
>  }
> +updateSortedActivityList();
>  

You can just find the activity in the list, and remove it - the order for the 
rest will not change.

> Activities_p.h:76
>  QHash activities;
> +QList sortedActivities;
>  QReadWriteLock activitiesLock;

`QList` -> `QVector`.

`QList` is an evil and slow class :)

REPOSITORY
  R161 KActivity Manager Service

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

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[CI System]

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
  [...truncated 220 lines...][2019-07-11T12:08:53.492Z] PROCESSOR_ARCHITECTURE= 'AMD64'[2019-07-11T12:08:53.492Z] PROCESSOR_IDENTIFIER  = 'Intel64 Family 6 Model 94 Stepping 3, GenuineIntel'[2019-07-11T12:08:53.492Z] PROCESSOR_LEVEL   = '6'[2019-07-11T12:08:53.492Z] PROCESSOR_REVISION= '5e03'[2019-07-11T12:08:53.492Z] PROGRAMDATA   = 'C:\ProgramData'[2019-07-11T12:08:53.492Z] PROGRAMFILES  = 'C:\Program Files'[2019-07-11T12:08:53.492Z] PROGRAMFILES(X86) = 'C:\Program Files (x86)'[2019-07-11T12:08:53.492Z] PROGRAMW6432  = 'C:\Program Files'[2019-07-11T12:08:53.492Z] PROMPT= '$P$G'[2019-07-11T12:08:53.492Z] PSMODULEPATH  = 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\'[2019-07-11T12:08:53.492Z] PUBLIC= 'C:\Users\Public'[2019-07-11T12:08:53.492Z] RUN_CHANGES_DISPLAY_URL   = 'https://build.kde.org/job/Plasma/job/drkonqi/job/kf5-qt5%20WindowsMSVCQt5.11/61/display/redirect?page=changes'[2019-07-11T12:08:53.492Z] RUN_DISPLAY_URL   = 'https://build.kde.org/job/Plasma/job/drkonqi/job/kf5-qt5%20WindowsMSVCQt5.11/61/display/redirect'[2019-07-11T12:08:53.492Z] STAGE_NAME= 'Configuring Build'[2019-07-11T12:08:53.492Z] SYSTEMDRIVE   = 'C:'[2019-07-11T12:08:53.492Z] SYSTEMROOT= 'C:\WINDOWS'[2019-07-11T12:08:53.492Z] TEMP  = 'C:\Users\Jenkins\AppData\Local\Temp'[2019-07-11T12:08:53.492Z] TMP   = 'C:\Users\Jenkins\AppData\Local\Temp'[2019-07-11T12:08:53.492Z] UCRTVERSION   = '10.0.17763.0'[2019-07-11T12:08:53.492Z] UNIVERSALCRTSDKDIR= 'C:\Program Files (x86)\Windows Kits\10\'[2019-07-11T12:08:53.492Z] USERDOMAIN= 'DESKTOP-UA3NMTP'[2019-07-11T12:08:53.492Z] USERNAME  = 'Jenkins'[2019-07-11T12:08:53.492Z] USERPROFILE   = 'C:\Users\Jenkins'[2019-07-11T12:08:53.492Z] VCIDEINSTALLDIR   = 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\Common7\IDE\VC\'[2019-07-11T12:08:53.492Z] VCINSTALLDIR  = 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\'[2019-07-11T12:08:53.492Z] VCTOOLSINSTALLDIR = 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.16.27023\'[2019-07-11T12:08:53.492Z] VCTOOLSREDISTDIR  = 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Redist\MSVC\14.20.27508\'[2019-07-11T12:08:53.492Z] VCTOOLSVERSION= '14.16.27023'[2019-07-11T12:08:53.492Z] VISUALSTUDIOVERSION   = '16.0'[2019-07-11T12:08:53.492Z] VS140COMNTOOLS= 'C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\'[2019-07-11T12:08:53.492Z] VS160COMNTOOLS= 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\Common7\Tools\'[2019-07-11T12:08:53.492Z] VSCMD_ARG_APP_PLAT= 'Desktop'[2019-07-11T12:08:53.492Z] VSCMD_ARG_HOST_ARCH   = 'x64'[2019-07-11T12:08:53.492Z] VSCMD_ARG_TGT_ARCH= 'x64'[2019-07-11T12:08:53.493Z] VSCMD_ARG_VCVARS_VER  = '14.16'[2019-07-11T12:08:53.493Z] VSCMD_VER = '16.0.3'[2019-07-11T12:08:53.493Z] VSINSTALLDIR  = 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\'[2019-07-11T12:08:53.493Z] WINDIR= 'C:\WINDOWS'[2019-07-11T12:08:53.493Z] WINDOWSLIBPATH= 'C:\Program Files (x86)\Windows Kits\10\UnionMetadata\10.0.17763.0;C:\Program Files (x86)\Windows Kits\10\References\10.0.17763.0'[2019-07-11T12:08:53.493Z] WINDOWSSDKBINPATH = 'C:\Program Files (x86)\Windows Kits\10\bin\'[2019-07-11T12:08:53.493Z] WINDOWSSDKDIR = 'C:\Program Files (x86)\Windows Kits\10\'[2019-07-11T12:08:53.493Z] WINDOWSSDKLIBVERSION  = '10.0.17763.0\'[2019-07-11T12:08:53.493Z] WINDOWSSDKVERBINPATH  = 'C:\Program Files (x86)\Windows Kits\10\bin\10.0.17763.0\'[2019-07-11T12:08:53.493Z] WINDOWSSDKVERSION = '10.0.17763.0\'[2019-07-11T12:08:53.493Z] WORKSPACE = 'C:\CI\workspace\Plasma\drkonqi\kf5-qt5 WindowsMSVCQt5.11'[2019-07-11T12:08:53.493Z] __DOTNET_ADD_64BIT= '1'[2019-07-11T12:08:53.493Z] __DOTNET_PREFERRED_BITNESS = '64'[2019-07-11T12:08:53.493Z] __VSCMD_PREINIT_PATH  = 'C:\Program Files (x86)\Common Files\Oracle\Java\javapath;C:\Program Files\Python36-32\Scripts\;C:\Program Files\Python36-32\;C:\ProgramData\Oracle\Java\javapath;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\Program Files\Git\cmd;C:\WINDOWS\System32\OpenSSH\;C:\Users\Jenkins\AppData\Local\Microsoft\WindowsApps'[2019-07-11T12:08:53.493Z] CMAKE_PREFIX_PAT

D22393: remove UnhandledErrorDialog

[Harald Sitter]

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://phabricator.kde.org/D22393

AFFECTED FILES
  src/bugzillaintegration/bugzillalib.h
  src/bugzillaintegration/reportassistantpages_bugzilla.cpp
  src/bugzillaintegration/reportassistantpages_bugzilla.h
  src/bugzillaintegration/reportinterface.h

To: sitter, #plasma, apol
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22396: Fix typo in CREATE_GETTER_AND_SETTER

[Christian Muehlhaeuser]

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

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

AFFECTED FILES
  src/service/Activities.cpp

To: muesli, ivan, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

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

[Aleix Pol Gonzalez]

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 we'd still be getting an odd ~500ms freeze randomly
  Or we can just decide to move blocking or heavy algorithms to separate 
threads and enjoy our multicore computers and a fluid experience.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Stefan Brüns]

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: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22394: Polish IPv4 settings dialog

[Aleix Pol Gonzalez]

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

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

AFFECTED FILES
  libs/editor/settings/ipv4widget.cpp

To: apol, #plasma, jgrulich
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22396: Fix typo in CREATE_GETTER_AND_SETTER

[Christian Muehlhaeuser]

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
  undef-typo (branched from master)

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

AFFECTED FILES
  src/service/Activities.cpp

To: muesli, ivan
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22381: Add previous-/nextActivity methods

[Christian Muehlhaeuser]

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

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22381?vs=61517&id=61579

BRANCH
  prevnext-activity (branched from master)

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

AFFECTED FILES
  src/common/dbus/org.kde.ActivityManager.Activities.xml
  src/service/Activities.cpp
  src/service/Activities.h
  src/service/Activities_p.h

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22034: Introduce ContainmentLayoutManager QML plugin

[Marco Martin]

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
  - new behavior for resize handles

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22034?vs=60459&id=61576

BRANCH
  mart/containmentlayoutmanager

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

AFFECTED FILES
  components/CMakeLists.txt
  components/containmentlayoutmanager/CMakeLists.txt
  components/containmentlayoutmanager/abstractlayoutmanager.cpp
  components/containmentlayoutmanager/abstractlayoutmanager.h
  components/containmentlayoutmanager/appletcontainer.cpp
  components/containmentlayoutmanager/appletcontainer.h
  components/containmentlayoutmanager/appletslayout.cpp
  components/containmentlayoutmanager/appletslayout.h
  components/containmentlayoutmanager/configoverlay.cpp
  components/containmentlayoutmanager/configoverlay.h
  components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp
  components/containmentlayoutmanager/containmentlayoutmanagerplugin.h
  components/containmentlayoutmanager/gridlayoutmanager.cpp
  components/containmentlayoutmanager/gridlayoutmanager.h
  components/containmentlayoutmanager/itemcontainer.cpp
  components/containmentlayoutmanager/itemcontainer.h
  components/containmentlayoutmanager/qml/BasicAppletContainer.qml
  components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml
  components/containmentlayoutmanager/qml/PlaceHolder.qml
  components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml
  components/containmentlayoutmanager/qml/qmldir
  components/containmentlayoutmanager/resizehandle.cpp
  components/containmentlayoutmanager/resizehandle.h

To: mart, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22035: Port FolderView to ContainmentLayoutManager plugin

[Marco Martin]

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.org/D22035

AFFECTED FILES
  containments/desktop/package/contents/ui/ActionButton.qml
  containments/desktop/package/contents/ui/AppletAppearance.qml
  containments/desktop/package/contents/ui/AppletHandle.qml
  containments/desktop/package/contents/ui/ConfigOverlay.qml
  containments/desktop/package/contents/ui/ResizeHandle.qml
  containments/desktop/package/contents/ui/code/LayoutManager.js
  containments/desktop/package/contents/ui/main.qml
  containments/panel/contents/ui/ConfigOverlay.qml
  desktoppackage/contents/updates/move_desktop_layout_config.js

To: mart, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22394: Polish IPv4 settings dialog

[Aleix Pol Gonzalez]

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 hostname 
is empty.
  Show an specialValueText and a suffix on the timeout field.
  Make sure we don't try to save the dialog if the parent window has been 
deleted.

TEST PLAN
  Used it

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  master

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

AFFECTED FILES
  libs/editor/settings/ipv4widget.cpp

To: apol, #plasma, jgrulich
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22393: remove UnhandledErrorDialog

[Harald Sitter]

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. it was used for when random unexpected HTML
  would get returned. there is no longer unexpected html:
  
  - the api always returns json blobs
  - should the api crap out we'll get protocol errors (HTTP 500 or some such) 
which is also forced into a serialized form and raised as a ProtocolException 
to be handled like errors from the API directly
  
  we always get one "standardized" error message and never an
  extended html to display.

TEST PLAN
  builds

REPOSITORY
  R871 DrKonqi

BRANCH
  remove-unhandlederrordialog

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

AFFECTED FILES
  src/bugzillaintegration/bugzillalib.h
  src/bugzillaintegration/reportassistantpages_bugzilla.cpp
  src/bugzillaintegration/reportassistantpages_bugzilla.h
  src/bugzillaintegration/reportinterface.h

To: sitter, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22344: Expose some more settings in an Advanced dialog

[Kai Uwe Broulik]

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
> +
> +connect(dlg, &QDialog::accepted,
> +[&] () {

Missing a `this` context

REPOSITORY
  R116 Plasma Network Management Applet

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

To: apol, #plasma, jgrulich
Cc: aspotashev, broulik, ngraham, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22344: Expose some more settings in an Advanced dialog

[Aleix Pol Gonzalez]

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=61520&id=61573

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

AFFECTED FILES
  libs/editor/settings/ipv4widget.cpp
  libs/editor/settings/ipv4widget.h
  libs/editor/settings/ui/ipv4.ui

To: apol, #plasma, jgrulich
Cc: aspotashev, broulik, ngraham, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22322: Store crash report automatically if shutting down

[Harald Sitter]

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 brace goes on next line for multi-line 
function bodies

> drkonqi.cpp:217
> +  QDir::SortFlag::Time | 
> QDir::Reversed);
> +if (fileList.size() >= 10) {
> +int filesToRemove = fileList.size() - 9;

This doesn't necessarily need changing, but I want to point out that this and 
the following two lines are convoluted, they are simply `for (int i = 
fileList.size(); i <= 10; --i) {}` if I am reading this right.

> drkonqi.cpp:219
> +int filesToRemove = fileList.size() - 9;
> +while(filesToRemove--) {
> +auto currentFile = fileList.takeFirst();

space between while and brace

> drkonqi.cpp:232
> +QDir dir(dirname);
> +if(!dir.mkpath(dirname)) {
> +qApp->quit();

space between if and brace

REPOSITORY
  R871 DrKonqi

BRANCH
  storeGuiless

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

To: tcanabrava, sitter, davidedmundson
Cc: sitter, davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Jonathan Marten]

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: ltoscano, kde-frameworks-devel, plasma-devel, fprice, LeGast00n, jraleigh, 
fbampaloukas, alexde, GB_2, feverfew, ragreen, Pitel, meven, michaelh, spoorun, 
navarromorales, ZrenBot, firef, ngraham, andrebarros, bruns, himcesjf, 
emmanuelp, lesliezhai, ali-mohamed, mikesomov, jensreuterberg, abetts, sebas, 
apol, mart