From kde-panel-devel Sat Dec 21 13:51:14 2013 From: "David Edmundson" Date: Sat, 21 Dec 2013 13:51:14 +0000 To: kde-panel-devel Subject: Re: Review Request 114566: Plasmate: Port remoteinstaller to Qt5/KF5 Message-Id: <20131221135114.22692.57869 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=138763388830350 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1318804668791352459==" --===============1318804668791352459== Content-Type: multipart/alternative; boundary="===============0245531092616189540==" --===============0245531092616189540== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114566/#review45989 ----------------------------------------------------------- plasmate/CMakeLists.txt No point committing commented code. plasmate/publisher/remoteinstaller/remoteinstaller.cpp This seems wrong. from kde4support/src/kio/netaccess.h static KDE4SUPPORT_DEPRECATED_EXPORT bool mkdir( const QUrl & url, QWidget* window, int permissions = -1 ); so you're casting book ok which is uninitialised to permissions - and then using that as the result. Maybe you can explain why you had to change it? plasmate/publisher/remoteinstaller/remoteinstallerdialog.cpp You can't make a layout on the stack. The lifetime of the layout needs to be as long as the RemoteInstallerDialog. This will be deleted as soon as we exit the constructor. - David Edmundson On Dec. 20, 2013, 4:02 p.m., Antonis Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/114566/ > ----------------------------------------------------------- > > (Updated Dec. 20, 2013, 4:02 p.m.) > > > Review request for Plasma and Giorgos Tsiapaliokas. > > > Repository: plasmate > > > Description > ------- > > Hello, > > This patch ports the remoteinstaller to Qt5/KF5. > > > Diffs > ----- > > plasmate/CMakeLists.txt 55e7453 > plasmate/publisher/remoteinstaller/remoteinstaller.cpp 5f4de97 > plasmate/publisher/remoteinstaller/remoteinstallerdialog.cpp 3b41b33 > > Diff: http://git.reviewboard.kde.org/r/114566/diff/ > > > Testing > ------- > > I wasn't able to test if it works. Because remoteinstaller is using the KUrlRequester, > which it segfaults... I have open a thread on the frameworks ML. > > > Thanks, > > Antonis Tsiapaliokas > > --===============0245531092616189540== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114566/

plasmate/CMakeLists.txt (Diff revision 1)
154	# ${KDE4_KDEUI_LIBS}	154	# ${KDE4_KDEUI_LIBS}

No point committing commented code.

plasmate/publisher/remoteinstaller/remoteinstaller.cpp (Diff revision 1)
		43	KIO::NetAccess::mkdir(tmpUrl, m_widget, ok);

This seems wrong.

from kde4support/src/kio/netaccess.h
    static KDE4SUPPORT_DEPRECATED_EXPORT bool mkdir( const QUrl & url, QWidget* window, int permissions = -1 );


so you're casting book ok which is uninitialised to permissions - and then using that as the result.

Maybe you can explain why you had to change it?

plasmate/publisher/remoteinstaller/remoteinstallerdialog.cpp (Diff revision 1)
		27	QVBoxLayout l;

You can't make a layout on the stack.

The lifetime of the layout needs to be as long as the RemoteInstallerDialog. This will be deleted as soon as we exit the constructor.

- David Edmundson

On December 20th, 2013, 4:02 p.m. UTC, Antonis Tsiapaliokas wrote:

Review request for Plasma and Giorgos Tsiapaliokas.

By Antonis Tsiapaliokas.

Updated Dec. 20, 2013, 4:02 p.m.

Repository: plasmate

Description

Hello,

This patch ports the remoteinstaller to Qt5/KF5.

Testing

I wasn't able to test if it works. Because remoteinstaller is using the KUrlRequester,
which it segfaults... I have open a thread on the frameworks ML.

Diffs

plasmate/CMakeLists.txt (55e7453)
plasmate/publisher/remoteinstaller/remoteinstaller.cpp (5f4de97)
plasmate/publisher/remoteinstaller/remoteinstallerdialog.cpp (3b41b33)

--===============0245531092616189540==-- --===============1318804668791352459== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel --===============1318804668791352459==--