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 l
On Thu, Aug 13, 2015 at 7:59 AM, Ivan Čukić 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 nature of KF
lasma/generic/runners/nepomuksearch/queryclientwrapper.cpp 0b828b0
Diff: https://git.reviewboard.kde.org/r/104526/diff/
Testing
---
Works in my Plasma Active installation.
Thanks,
Lamarque Souza
x27;s 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
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 w
Hi all,
Regarding ModemManagerQt, libbluedevil, libkscreen and baloo, they are
supposed to be frameworks stuff (not sure about baloo) but they are not
ready yet. Why not created a frameworks-next group to include them there
until they are ready to move to frameworks (being it KF5 or even KF6 in th
tps://git.reviewboard.kde.org/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
/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
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 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
> >
> 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
> >
before 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:
>
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 Amurar
12869/diff/
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
is, m_fileFormatVersion for instance.
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
r/popupproxy.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25728>
qobject_cast. I am uneasy in doing a cast and does not checking for the
returned value.
- Lamarque Souza
On May 15, 2013, 7:39 p.m., José Millán Soto wrote:
>
> -
t; and the pointer name. I am not sure if the maintainer of this
code is ok with mixing code style until everything is converted to the new one.
drkonqi/reportinterface.cpp
<http://git.reviewboard.kde.org/r/110687/#comment24624>
same here.
- Lamarque Souza
On May 28, 2013, 11:01 a
g/r/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 a
> 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
> > yo
rary QString.
- Lamarque Souza
On May 6, 2013, 4:01 p.m., Eike Hein wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.revi
> 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 ma
> 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 ge
> On Feb. 20, 2011, 3:51 p.m., Lukáš Tinkl wrote:
> > Looks OK to me
Can this patch be commited to trunk?
- Lamarque
---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6513/#review9893
-
26 matches
Mail list logo