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 /usr/incl

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 wrote: >This is an automatically generated e-mail. To reply,

Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Hrvoje Senjan
> On Oct. 19, 2014, 2:49 a.m., Hrvoje Senjan wrote: > > interfaces/kompareinterface.h, line 25 > > > > > > this include is also provided by KDELibs4Support.. > > > > quick grep shows it was also left in i

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 wrote: >This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120627/ >interfaces/kompareinterface.h >

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
--- 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 m

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, 3:03 p.m.) Review request for kdelibs and Kevin K

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 Kevin Kofler
> On Okt. 18, 2014, 4:40 vorm., Kevin Kofler wrote: > > komparepart/kompare_part.cpp, line 303 > > > > > > So where does this temporary file get deleted? Apparently nowhere. > > > > You have to handle th

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 > > > > > > So where does this temporary file get deleted? Apparently nowhere. > > > > You have to handle th

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
--- 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 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 komp

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 > > > > > > This line looks very wrong to me: You're executing the block only if > > the stat job failed? This needs

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 K

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 > > > > > > I think this now needs something like: > > QDir::isAbsolutePath(args.at(0)) ? QUrl::fromLocalFile(args.at(0)) : > > QU

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 > > > > > > I think this now needs something like: > > QDir::isAbsolutePath(args.at(0)) ? QUrl::fromLocalFile(args.at(0)) : > > QU

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 lifti

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 w

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 K

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 yea

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 ye

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 som

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 Descri