Re: Review Request 116951: Fix KDBusServiceStarter::findServiceFor() not returning error string
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116951/#review55915 --- This review has been submitted with commit fe8a0f456da5d3827e41c7754362844ebd11af84 by David Jarvie to branch KDE/4.12. - Commit Hook On April 14, 2014, 11:48 a.m., David Jarvie wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116951/ --- (Updated April 14, 2014, 11:48 a.m.) Review request for kdelibs. Repository: kdelibs Description --- When KDBusServiceStarter::findServiceFor() fails to start the requested service after it is found to not be running, it does not return the error string. This patch fixes that and makes it behave as in the apidox. Diffs - kio/kio/kdbusservicestarter.cpp 90624fb Diff: https://git.reviewboard.kde.org/r/116951/diff/ Testing --- Tested this scenario, and it now returns the error string. Thanks, David Jarvie
Re: Review Request 116951: Fix KDBusServiceStarter::findServiceFor() not returning error string
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116951/ --- (Updated April 16, 2014, 8:25 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Repository: kdelibs Description --- When KDBusServiceStarter::findServiceFor() fails to start the requested service after it is found to not be running, it does not return the error string. This patch fixes that and makes it behave as in the apidox. Diffs - kio/kio/kdbusservicestarter.cpp 90624fb Diff: https://git.reviewboard.kde.org/r/116951/diff/ Testing --- Tested this scenario, and it now returns the error string. Thanks, David Jarvie
Re: Kronometer now in KDE Review
2014-04-15 22:27 GMT+02:00 Albert Astals Cid aa...@kde.org: El Dilluns, 14 d'abril de 2014, a les 11:35:02, Elvis Angelaccio va escriure: 2014-04-14 1:06 GMT+02:00 Albert Astals Cid aa...@kde.org: * Your choice of splitters to separate hours/minutes/seconds seems a bit weird do you think that anyone will use it to have something like very wide minutes and narrow the rest? I see your point, probably the splitters are unnecessary UI components for this use case. What do you think about an option in Interface settings? I could display by default a single QFrame (without splitters) and leave to the user an opt-in to allow the splitters. In this way I can reuse the existing code without too much refactoring. I just don't see the need for the splitters, why would someone want to have them? To be honest there are no real motivations, I just thought that a splitter could be a further feature. But indeed it's useless, at least if not used for a particular use case (see Thomas suggestions). I've just pushed a test branch without splitters, for a quick feedback look here: http://abload.de/img/kronometer-no-splittevdklc.png If you want instead a single display without the dividers between hours/minutes/etc, then I can push another experimental branch test2. Something like this older version: http://abload.de/img/kronometer-running-la6ddg3.png But since now kronometer has those header labels above the numbers, I would need a tabular layout and from my earlier tests I remember that it looks ugly. Cheers, Albert Regards, Elvis
Re: Kronometer now in KDE Review
2014-04-16 11:41 GMT+02:00 Elvis Angelaccio elvis.angelac...@kdemail.net: 2014-04-15 22:27 GMT+02:00 Albert Astals Cid aa...@kde.org: El Dilluns, 14 d'abril de 2014, a les 11:35:02, Elvis Angelaccio va escriure: 2014-04-14 1:06 GMT+02:00 Albert Astals Cid aa...@kde.org: * Your choice of splitters to separate hours/minutes/seconds seems a bit weird do you think that anyone will use it to have something like very wide minutes and narrow the rest? I see your point, probably the splitters are unnecessary UI components for this use case. What do you think about an option in Interface settings? I could display by default a single QFrame (without splitters) and leave to the user an opt-in to allow the splitters. In this way I can reuse the existing code without too much refactoring. I just don't see the need for the splitters, why would someone want to have them? To be honest there are no real motivations, I just thought that a splitter could be a further feature. But indeed it's useless, at least if not used for a particular use case (see Thomas suggestions). I've just pushed a test branch without splitters, for a quick feedback look here: http://abload.de/img/kronometer-no-splittevdklc.png If you want instead a single display without the dividers between hours/minutes/etc, then I can push another experimental branch test2. Something like this older version: http://abload.de/img/kronometer-running-la6ddg3.png But since now kronometer has those header labels above the numbers, I would need a tabular layout and from my earlier tests I remember that it looks ugly. Sorry, forget my last statement. I've just tried using a QGridLayout and there is nothing wrong with it: http://abload.de/img/kronometer-no-divideri4kce.png This alternative is in the branch test2. I just need to fix the QTimeDisplay::setTimeFormat() function. So, the choice is between the branch test and test2. Let me know what do you prefer. For completeness the alternatives in details are: - branch test: no splitters, with dividers (i.e. 4 QFrames in a horizontal layout): http://abload.de/img/kronometer-no-splittevdklc.png - branch test2: no splitters, no dividers (i.e. single QFrame with a grid layout): http://abload.de/img/kronometer-no-divideri4kce.png
