D14437: Fix QFileDialog not remembering the last visited directory.

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> apol wrote in kdeplatformfiledialoghelper.cpp:118
> I'm not sure I understand the comment. Is it like a TODO?

It works without comment, i think these connections are not needed.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: dfaure, anthonyfieroni, elvisangelaccio, plasma-devel, broulik, arichardson
Cc: apol, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[Anthony Fieroni]

anthonyfieroni added a comment.


  +1 from me.

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

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14474: Stop enforcing spacings

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Yes yes yes yes!

REPOSITORY
  R134 Discover Software Store

BRANCH
  master

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

To: apol, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14437: Fix QFileDialog not remembering the last visited directory.

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:118
> +connect(m_fileWidget->dirOperator(), ::urlEntered, this, 
> ::directoryEntered);
> +// ## no connect to fileSelected, filesSelected, fileHighlighted, 
> currentChanged?
>  layout()->addWidget(m_buttons);

I'm not sure I understand the comment. Is it like a TODO?

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: dfaure, anthonyfieroni, elvisangelaccio, plasma-devel, broulik, arichardson
Cc: apol, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D14050: Fwupd Backend For Review and Improvement

[Aleix Pol Gonzalez]

apol added a comment.


  Please, do some review yourself, you should be able to see most of these 
things yourself

INLINE COMMENTS

> FwupdSourcesBackend.cpp:151
> +{
> +return  m_actions ;
> +}

`return {}` and remove the unused attribute.

> FwupdSourcesBackend.h:46
> +bool supportsAdding() const override { return false; }
> +void eulaRequired(const QString& remoteName , const QString& 
> licenseAgreement);
> +void populateSources();

There's no space before a coma.

> FwupdSourcesBackend.h:54
> +FwupdSourcesModel* m_sources;
> +QList m_actions;
> +};

Remove.

> FwupdTransaction.cpp:99
> +QNetworkAccessManager *manager = new QNetworkAccessManager(this);
> +connect(manager, SIGNAL(finished(QNetworkReply*)),this, 
> SLOT(fwupdInstall(QNetworkReply*)));
> +manager->get(QNetworkRequest(uri));

Use proper connect syntax.

> FwupdTransaction.cpp:160
> +
> +void FwupdTransaction::iterateTransaction()
> +{

Give it a name that says what it does, this is just copied from the dummy and 
thus it has a meaningless name.

REPOSITORY
  R134 Discover Software Store

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

To: abhijeet2096, apol, davidedmundson
Cc: zzag, anthonyfieroni, plasma-devel, ragreen, ixoos, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14474: Stop enforcing spacings

[Aleix Pol Gonzalez]

apol added a comment.


  F6161719: Screenshot_20180730_014620.png 


REPOSITORY
  R134 Discover Software Store

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

To: apol, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14474: Stop enforcing spacings

[Aleix Pol Gonzalez]

apol created this revision.
apol added a reviewer: ngraham.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
apol requested review of this revision.

REVISION SUMMARY
  Kirigami isn't designed to have this spacing between elements and it keeps
  changing what it looks like.
  
  BUG: 396895

REPOSITORY
  R134 Discover Software Store

BRANCH
  master

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

AFFECTED FILES
  discover/qml/UpdatesPage.qml

To: apol, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10040: Add serial number and EISA ID to OutputDevice interface

[David Edmundson]

davidedmundson updated this revision to Diff 38734.
davidedmundson added a comment.


  exciting whitespace change

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10040?vs=38733=38734

BRANCH
  output_changes

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

AFFECTED FILES
  autotests/client/test_wayland_outputdevice.cpp
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: davidedmundson, graesslin, sebas, #kwin, dvratil
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[David Edmundson]

davidedmundson updated this revision to Diff 38733.
davidedmundson added a comment.


  Instead of modifying the geometry event use two new events to remain
  fully compatiable.

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10040?vs=25806=38733

BRANCH
  output_changes

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

AFFECTED FILES
  autotests/client/test_wayland_outputdevice.cpp
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: davidedmundson, graesslin, sebas, #kwin, dvratil
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D14436: SwitchDesktop mousewheel options with config dialog added

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  Showing a disabled checkbox for rollover doesn't make much sense.
  IMHO we should either have it as a separate option from kwin and have a 
checkbox, or not have a checkbox at all.

INLINE COMMENTS

> totto wrote in desktop.cpp:36
> The rollover option is from another kde component (kwin), is there a way to 
> get notified whenever this config changes?

I have this coming: 
https://phabricator.kde.org/D13034

> desktop.cpp:122
> +QCheckBox *item = new QCheckBox(widget);
> +item->setText(i18nc(e.cfgKey.toStdString().c_str(),
> +e.name.toStdString().c_str()));

Unforunately, that's not how our i18n works.

a script greps for i18n("some text here") to generate the english translations.

> desktop.h:54
>  
> +// QSharedPtr has non-explicit operator bool which is bugprone, work 
> around that
> +struct explicitBool { bool val; };

or just use a regular bool and copy the value in configurationAccepted

Long term, having simpler code tends to be much better than clever code.

REPOSITORY
  R120 Plasma Workspace

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

To: totto, hein, broulik, #plasma, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12708: Only include QtQuick support in Breeze KStyle if QtQuick is available

[Alexander Schlarb]

tundracomp added a comment.


  That would be „Alexander Schlarb “. Thanks!

REPOSITORY
  R31 Breeze

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

To: tundracomp, mart
Cc: ngraham, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart