Hi,
My review:
. there are several code style inconsistencies, like "QDialog* parent" and
"Ui::AppChooserDialog * m_dialog". In some places you use app_id while in
other places you use appId.
. you use 0 in same places that you should use nullptr.
. there is no doxygen documentation at all.
.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128750/#review99830
---
Ship it!
Ship It!
- Lamarque Souza
On Aug. 24, 2016
I agree but there is a problem: it can catch a lot of errors in our
dependency libraries (upstream bugs). I had this problem when I used it
with a program I develop at my work. Enabling it for all programs at once
and fixing all those upstream bugs can be overwhelming. Maybe we should do
it for a
On Thu, Aug 13, 2015 at 7:59 AM, Ivan Čukić ivan.cu...@kde.org wrote:
I prefer the first option, it's the way forward and if someone was using
I'd say that requiring a newer gcc just like that would go against the
nature of the KF5 project.
I don't really see why it is against the
like fish and
nfs create symlinks. I am also working on some other changes that could use
this feature.
Diffs
-
kio/kio/copyjob.cpp 8dde763
Diff: https://git.reviewboard.kde.org/r/105831/diff/
Testing
---
Thanks,
Lamarque Souza
/queryclientwrapper.cpp 0b828b0
Diff: https://git.reviewboard.kde.org/r/104526/diff/
Testing
---
Works in my Plasma Active installation.
Thanks,
Lamarque Souza
ModemManagerQt is used by two KDE Applications, Plasma NM and KDE
Telepathy. The former uses it to retrieve information about mobile
broadband connections (operator and signal quality), unlocking modem's sim
card when activating 3G connections, listing available modems in mobile
connection wizard
/r/120573/#comment47629
QFile::remove() does not work with directories. You should use
QDir().rmdir() instead, which only works as long as the directory is empty
though.
- Lamarque Souza
On Oct. 13, 2014, 5:32 p.m., René J.V. Bertin wrote
/powerdevilupowerbackend.cpp
https://git.reviewboard.kde.org/r/119088/#comment42813
A line with 'delete job' before this return is missing here.
- Lamarque Souza
On July 2, 2014, 5:45 p.m., Martin Bříza wrote:
---
This is an automatically generated e-mail
Hi all,
I agree that Plasma NM should go to kde/workspace. In the past the only
thing that prevented that was the fact that we needed to change
translatable strings from time to time, which obviously does not comply
with the string freeze. Now that kde/workspace is released more often that
should
on reviewboard. Can you
provide the correct patch?
I looked into the raw patch and I think the return 1 line that you commented
should be kept when the action is not upgrade.
- Lamarque Souza
On March 30, 2014, 3:47 a.m., Andrei Amuraritei wrote
the mentioned return 1
line, otherwise we will have a memory leak.
- Lamarque Souza
On March 30, 2014, 3:47 a.m., Andrei Amuraritei wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117175
On March 30, 2014, 11:23 a.m., Lamarque Souza wrote:
Hi, there is an error when trying to show the patch on reviewboard. Can you
provide the correct patch?
I looked into the raw patch and I think the return 1 line that you
commented should be kept when the action is not upgrade
On March 30, 2014, 11:23 a.m., Lamarque Souza wrote:
Hi, there is an error when trying to show the patch on reviewboard. Can you
provide the correct patch?
I looked into the raw patch and I think the return 1 line that you
commented should be kept when the action is not upgrade
/
Testing (updated)
---
Tested by the bug reporter and it fixes the problem for him.
Thanks,
Lamarque Souza
.
http://bugs.kde.org/show_bug.cgi?id=324954
Diffs
-
solid/solid/backends/udev/udevnetworkinterface.cpp 06dc907
Diff: http://git.reviewboard.kde.org/r/112869/diff/
Testing
---
No testing but it is pretty obvious that we should not left that socket opened.
Thanks,
Lamarque Souza
for the
returned value.
- Lamarque Souza
On May 15, 2013, 7:39 p.m., José Millán Soto wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110459
.
Shouldn't this variable go to version.h? klipper_version is defined there.
- Lamarque Souza
On May 15, 2013, 7:39 p.m., José Millán Soto wrote:
---
This is an automatically generated e-mail. To reply, visit:
http
.
drkonqi/reportinterface.cpp
http://git.reviewboard.kde.org/r/110687/#comment24624
same here.
- Lamarque Souza
On May 28, 2013, 11:01 a.m., Jekyll Wu wrote:
---
This is an automatically generated e-mail. To reply, visit:
http
/110670/#comment24571
What is exactly the bug this patch is trying to solve? Is there any bug
entry in bugzilla for it (or them)?
- Lamarque Souza
On May 27, 2013, 1:14 p.m., Oliver Henshaw wrote:
---
This is an automatically
[2] https://git.reviewboard.kde.org/r/110331
kwalletd/kwalletd.cpp
http://git.reviewboard.kde.org/r/110328/#comment23931
password is a QString, right? Then you should use password.clear() here
instead of assigning an empty QString. That avoids creating a temporary QString.
- Lamarque
On May 6, 2013, 4:40 p.m., Lamarque Souza wrote:
kwalletd/kwalletd.h, line 226
http://git.reviewboard.kde.org/r/110328/diff/1/?file=142372#file142372line226
Why split this feature into three different review requests? One of
your requests [2] is just one line change and the other
On Feb. 20, 2011, 3:51 p.m., Lukáš Tinkl wrote:
Looks OK to me
Lamarque Souza wrote:
Can this patch be commited to trunk?
Lamarque Souza wrote:
Ok, I will commit this patch then.
Patch submitted. I do not know why but it has not been marked as so. Someone
please close
On Feb. 20, 2011, 3:51 p.m., Lukáš Tinkl wrote:
Looks OK to me
Lamarque Souza wrote:
Can this patch be commited to trunk?
Ok, I will commit this patch then.
- Lamarque
---
This is an automatically generated e-mail. To reply
24 matches
Mail list logo
