From kwrite-devel Mon Dec 23 10:09:37 2013 From: "Michal Humpula" Date: Mon, 23 Dec 2013 10:09:37 +0000 To: kwrite-devel Subject: Re: Review Request 114551: frameworks: port from KIO::NetAccess Message-Id: <20131223100937.18783.99597 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kwrite-devel&m=138779339323719 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============2708610156633334088==" --===============2708610156633334088== Content-Type: multipart/alternative; boundary="===============4192223000309827554==" --===============4192223000309827554== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Dec. 23, 2013, 8:21 a.m., David Faure wrote: > > part/document/katedocument.cpp, line 2190 > > > > > > backupSuccess = job->exec(); would be shorter, but yeah, this is equivalent. > > I think it's more readable to see immediately that the return value of exec() is checked, rather than looking further down for a check on error() [this applies to the other parts of the patch too]. Thanks for input! I will adjust the places appropriately. - Michal ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114551/#review46064 ----------------------------------------------------------- On Dec. 21, 2013, 10:08 p.m., Michal Humpula wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114551/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2013, 10:08 p.m.) > > > Review request for Kate and David Faure. > > > Repository: kate > > > Description > ------- > > KIO::NetAccess made it to kde4support, so this is the patch to port from it. > > This is my first touch with KIO, so need someone to recheck if it makes sense. Apps are running ok it seems:) > > > Diffs > ----- > > kate/app/katesession.cpp 17d8741 > kwrite/kwritemain.cpp fc14fda > part/dialogs/katedialogs.cpp c536d3d > part/document/katedocument.cpp 286c061 > > Diff: https://git.reviewboard.kde.org/r/114551/diff/ > > > Testing > ------- > > > Thanks, > > Michal Humpula > > --===============4192223000309827554== 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: https://git.reviewboard.kde.org/r/114551/

On December 23rd, 2013, 8:21 a.m. UTC, David Faure wrote:

part/document/katedocument.cpp (Diff revision 1)
bool KateDocument::saveFile()
		2190	backupSuccess = !job->error();

backupSuccess = job->exec(); would be shorter, but yeah, this is equivalent.
I think it's more readable to see immediately that the return value of exec() is checked, rather than looking further down for a check on error() [this applies to the other parts of the patch too].

Thanks for input! I will adjust the places appropriately.

- Michal

On December 21st, 2013, 10:08 p.m. UTC, Michal Humpula wrote:

Review request for Kate and David Faure.

By Michal Humpula.

Updated Dec. 21, 2013, 10:08 p.m.

Repository: kate

Description

KIO::NetAccess made it to kde4support, so this is the patch to port from it.

This is my first touch with KIO, so need someone to recheck if it makes sense. Apps are running ok it seems:)

Diffs

kate/app/katesession.cpp (17d8741)
kwrite/kwritemain.cpp (fc14fda)
part/dialogs/katedialogs.cpp (c536d3d)
part/document/katedocument.cpp (286c061)

--===============4192223000309827554==-- --===============2708610156633334088== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ KWrite-Devel mailing list KWrite-Devel@kde.org https://mail.kde.org/mailman/listinfo/kwrite-devel --===============2708610156633334088==--