Re: Review Request 120627: Remove kdelibs4support.

2014-10-21 Thread Kevin Kofler
On Monday 20 October 2014 at 20:53:51, Jeremy Whiting wrote: Thanks again, another review just posted. Guess I need to port applications on a vm that doesn't have kdelibs4 installed to make sure I get these right and complete. Or use a Fedora machine, our kdelibs4 headers are under

Re: Review Request 120627: Remove kdelibs4support.

2014-10-20 Thread Jeremy Whiting
Hrvoje, Thanks again, another review just posted. Guess I need to port applications on a vm that doesn't have kdelibs4 installed to make sure I get these right and complete. Jeremy On Sun, Oct 19, 2014 at 12:49 AM, Hrvoje Senjan hrvoje.sen...@gmail.com wrote: This is an automatically

Re: Review Request 120627: Remove kdelibs4support.

2014-10-19 Thread Hrvoje Senjan
On Oct. 19, 2014, 2:49 a.m., Hrvoje Senjan wrote: interfaces/kompareinterface.h, line 25 https://git.reviewboard.kde.org/r/120627/diff/6/?file=320653#file320653line25 this include is also provided by KDELibs4Support.. quick grep shows it was also left in

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Kevin Kofler
On Okt. 18, 2014, 4:40 vorm., Kevin Kofler wrote: komparepart/kompare_part.cpp, line 303 https://git.reviewboard.kde.org/r/120627/diff/3/?file=320382#file320382line303 So where does this temporary file get deleted? Apparently nowhere. You have to handle this the same

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Kevin Kofler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68660 --- komparepart/kompare_part.cpp

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- (Updated Oct. 18, 2014, 3:03 p.m.) Review request for kdelibs and Kevin

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Kevin Kofler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68677 --- Ship it! Looks good now. - Kevin Kofler On Okt. 18, 2014,

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- (Updated Oct. 18, 2014, 10:13 p.m.) Status -- This change has been

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Hrvoje Senjan
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68684 --- interfaces/kompareinterface.h

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Jeremy Whiting
Good point, I've posted another review that fixes this. On Sat, Oct 18, 2014 at 6:49 PM, Hrvoje Senjan hrvoje.sen...@gmail.com wrote: This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ interfaces/kompareinterface.h

Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- Review request for kdelibs and Kevin Kofler. Repository: kompare

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68622 --- Use QLayout/QFrame instead of KVBox (seems broken though

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
On Okt. 17, 2014, 6:03 nachm., Kevin Kofler wrote: Use QLayout/QFrame instead of KVBox (seems broken though somehow) Then I suggest you either fix it, or submit only the parts of the port that work. We have time until KF6 to port away from kdelibs4support, that's years ahead.

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
On Oct. 17, 2014, 12:03 p.m., Kevin Kofler wrote: Use QLayout/QFrame instead of KVBox (seems broken though somehow) Then I suggest you either fix it, or submit only the parts of the port that work. We have time until KF6 to port away from kdelibs4support, that's years ahead.

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- (Updated Oct. 17, 2014, 3:38 p.m.) Review request for kdelibs and Kevin

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68628 --- (Review of revision 1, because you posted revision 2 while I

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68636 --- Since the KVBox issue should be fixed in revision 2, I'm

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
On Okt. 17, 2014, 10:12 nachm., Kevin Kofler wrote: main.cpp, line 132 https://git.reviewboard.kde.org/r/120627/diff/1/?file=320275#file320275line132 I think this now needs something like: QDir::isAbsolutePath(args.at(0)) ? QUrl::fromLocalFile(args.at(0)) : QUrl(args.at(0))

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- (Updated Oct. 17, 2014, 9:40 p.m.) Review request for kdelibs and Kevin

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
On Oct. 17, 2014, 4:12 p.m., Kevin Kofler wrote: komparepart/kompare_part.cpp, line 295 https://git.reviewboard.kde.org/r/120627/diff/1/?file=320254#file320254line295 This line looks very wrong to me: You're executing the block only if the stat job failed? This needs at least the

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/#review68642 --- I wonder about the Qt 5.4 check though. This version of

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- (Updated Oct. 17, 2014, 10:50 p.m.) Review request for kdelibs and Kevin

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120627/ --- (Updated Oct. 17, 2014, 11:01 p.m.) Review request for kdelibs and Kevin

Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Jeremy Whiting
On Oct. 17, 2014, 10:40 p.m., Kevin Kofler wrote: komparepart/kompare_part.cpp, line 303 https://git.reviewboard.kde.org/r/120627/diff/3/?file=320382#file320382line303 So where does this temporary file get deleted? Apparently nowhere. You have to handle this the same