Re: Review Request 111648: Fix for GroupExpander that overlaps application icon
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111648/ --- (Updated July 23, 2013, 5:37 a.m.) Review request for kde-workspace and Eike Hein. Description --- In current task manager Arrow for the GroupExpander overlaps the application icon, Fix for it. Diffs - plasma/desktop/applets/tasks/package/contents/ui/GroupExpanderOverlay.qml 7aa0d00 Diff: http://git.reviewboard.kde.org/r/111648/diff/ Testing (updated) --- Works for me Thanks, Bhushan Shah
Review Request 111648: Fix for GroupExpander that overlaps application icon
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111648/ --- Review request for kde-workspace and Eike Hein. Description --- In current task manager Arrow for the GroupExpander overlaps the application icon, Fix for it. Diffs - plasma/desktop/applets/tasks/package/contents/ui/GroupExpanderOverlay.qml 7aa0d00 Diff: http://git.reviewboard.kde.org/r/111648/diff/ Testing --- Thanks, Bhushan Shah
Re: Review Request 111462: Avoid unnecessary stat of destination directory during copy/move in KDirWatch
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111462/ --- (Updated July 23, 2013, 3:47 a.m.) Review request for kdelibs and David Faure. Changes --- Updated patch and description of the fix. Description (updated) --- The patch has been modified to only avoid unnecessary stat of the destination directory during copy/move operations in KDirWatch. This way when copying a file A to directory B does not result in directory B being stat'ed everytime file A changes. Diffs (updated) - kdecore/io/kdirwatch.cpp a325f0f Diff: http://git.reviewboard.kde.org/r/111462/diff/ Testing --- - strace an instance of Konqueror or Dolphin. - copy a file from a remote location (ftp,sftp, smb) to your local file system. - check if there is a flood of stat calls on both the file being copied and its destination directory. Thanks, Dawit Alemayehu
Re: Review Request 111462: Avoid unnecessary stat of destination directory during copy/move in KDirWatch
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111462/#review36330 --- What if e->dirty was already true before entering the whole function? I think you'll have to save the old value before the e->dirty=true statement, and test it near your new code. Looks good otherwise. - David Faure On July 14, 2013, 11:57 p.m., Dawit Alemayehu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111462/ > --- > > (Updated July 14, 2013, 11:57 p.m.) > > > Review request for kdelibs and David Faure. > > > Description > --- > > The attached patch changes the logic that attempts to deal with a flood of > the dirty signals received by KDirLister when existing files are change. The > current code simply puts the file in an update pending list and starts a > delayed timer (500 ms) to process the request. As a result, every half a > second or so the pending update request is processed. This would have been > fine were it not for the fact that the processing results in a call to > KFileItem::refersh which does a stat the local file. Hence, a stat is done on > the file being downloaded every 500 ms. Additionally, some other code is also > doing a stat every half second on the download destination directory as well. > > This patch attempts to prevent the flood of stat calls by restarting the > update timer if we get another dirty signal on a file that is already in the > update pending list. IOW, we only do the last stat call. The downside of > doing that is the information about the file being download will be stale > until the download finishes. Unfortunately I still have not been able to > figure out what is doing the stat on the destination directory itself and > hence do not have a solution for that yet. Any ideas? > > > Diffs > - > > kdecore/io/kdirwatch.cpp a325f0f > > Diff: http://git.reviewboard.kde.org/r/111462/diff/ > > > Testing > --- > > - strace an instance of Konqueror or Dolphin. > - copy a file from a remote location (ftp,sftp, smb) to your local file > system. > - check if there is a flood of stat calls on both the file being copied and > its destination directory. > > > Thanks, > > Dawit Alemayehu > >
Re: Review Request 111633: Update announced KDE version / required Qt version
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111633/#review36326 --- Ship it! Yes, it was simply forgotten. - Alexander Neundorf On July 21, 2013, 4:40 p.m., Christoph Feck wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111633/ > --- > > (Updated July 21, 2013, 4:40 p.m.) > > > Review request for Build System, kdelibs and Alexander Neundorf. > > > Description > --- > > Not sure at which point in time we update the KDE version. > > Additionally, I changed the required Qt version, in the hope that build > system maintainers find more spots where version updates might be required. > > > Diffs > - > > cmake/modules/FindKDE4Internal.cmake 98a6f48 > > Diff: http://git.reviewboard.kde.org/r/111633/diff/ > > > Testing > --- > > > Thanks, > > Christoph Feck > >
Re: Review Request 111480: Notifications are shown multiple times
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111480/#review36320 --- This breaks Amarok's "Show OSD" shortcut where you could tell it to show the currently played track again. Imho this should somehow be fixed in Amarok, it is the only app I know that has this issue. - Kai Uwe Broulik On July 16, 2013, 12:01 p.m., Cedric Bellegarde wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111480/ > --- > > (Updated July 16, 2013, 12:01 p.m.) > > > Review request for kde-workspace and Marco Martin. > > > Description > --- > > Do not replace notifications with same summary and body > > Here a patch similar to this colibri commit. > > See REVIEW: 110745 > http://quickgit.kde.org/?p=colibri.git&a=commit&h=9a96b9512579215bcddd8fc88041fdd7130dbb0f > > > This addresses bug 313440. > http://bugs.kde.org/show_bug.cgi?id=313440 > > > Diffs > - > > plasma/generic/applets/notifications/contents/ui/Notifications.qml ce3f8de > > Diff: http://git.reviewboard.kde.org/r/111480/diff/ > > > Testing > --- > > Working here > > > Thanks, > > Cedric Bellegarde > >
Re: Review Request 111072: Drop internal X11 cmake module
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111072/#review36317 --- This review has been submitted with commit 1822b197d9cbfe94435355f9b68d34b6600df538 by Andrea Scarpino to branch frameworks. - Commit Hook On June 17, 2013, 3:35 p.m., Andrea Scarpino wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111072/ > --- > > (Updated June 17, 2013, 3:35 p.m.) > > > Review request for kdelibs and Alexander Neundorf. > > > Description > --- > > Use upstream one. > > > Diffs > - > > cmake/modules/CMakeLists.txt 25ea009 > cmake/modules/FindX11.cmake f2f879d > > Diff: http://git.reviewboard.kde.org/r/111072/diff/ > > > Testing > --- > > > Thanks, > > Andrea Scarpino > >
Re: Review Request 111072: Drop internal X11 cmake module
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111072/ --- (Updated July 22, 2013, 6:44 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs and Alexander Neundorf. Description --- Use upstream one. Diffs - cmake/modules/CMakeLists.txt 25ea009 cmake/modules/FindX11.cmake f2f879d Diff: http://git.reviewboard.kde.org/r/111072/diff/ Testing --- Thanks, Andrea Scarpino
Re: Supported Compilers / C++11 Support in KF5
On Mon, July 22, 2013 08:38:12 Johannes Sixt wrote: > Am 7/21/2013 13:52, schrieb Rolf Eike Beer: > > Explicit virtual overrides require g++ 4.7: > > > > http://gcc.gnu.org/projects/cxx0x.html > > > > This is trivially to work around by a CMake time check and then just > > define > > override to empty. > > Not so. 'override' is a legal identifier, not a pure keyword. By defining > it away, you break existing code that uses the name for something else. That's a great point (something I just came across in Stroustroup's 4th Edition of TC++PL the other day). 'override' and 'final' are both what are described as "contextual keywords" due to backward compatibility concerns. This means it's not safe to use pre-processor macros to expand either of those to be empty as they can be used in normal code sections (even though that's probably a bad idea at this point). Regards, - Michael Pyne signature.asc Description: This is a digitally signed message part.
Re: Review Request 111633: Update announced KDE version / required Qt version
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111633/#review36316 --- Ship it! Ok, you convinced me. Let's go for it. - Albert Astals Cid On July 21, 2013, 4:40 p.m., Christoph Feck wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111633/ > --- > > (Updated July 21, 2013, 4:40 p.m.) > > > Review request for Build System, kdelibs and Alexander Neundorf. > > > Description > --- > > Not sure at which point in time we update the KDE version. > > Additionally, I changed the required Qt version, in the hope that build > system maintainers find more spots where version updates might be required. > > > Diffs > - > > cmake/modules/FindKDE4Internal.cmake 98a6f48 > > Diff: http://git.reviewboard.kde.org/r/111633/diff/ > > > Testing > --- > > > Thanks, > > Christoph Feck > >
Re: Supported Compilers / C++11 Support in KF5
Am Montag, 22. Juli 2013, 19:14:10 schrieb Alexander Neundorf: > On Sunday 21 July 2013, Rolf Eike Beer wrote: > > I totally agree with that. As I said: detection of this is trivial at > > CMake time, maybe I get my C++ feature detection package ready even to be > > included in 2.8.12, and if not the stuff can easily be extracted. And > > Yes, that would be great :-) > You still wanted to have a rweview, right ? Sure. ;) Eike signature.asc Description: This is a digitally signed message part.
Re: Supported Compilers / C++11 Support in KF5
On Sunday 21 July 2013, Rolf Eike Beer wrote: > Am Sonntag, 21. Juli 2013, 14:11:07 schrieb Volker Krause: > > On Sunday 21 July 2013 13:52:06 Rolf Eike Beer wrote: > > > Volker Krause wrote: > > > > - GCC >= 4.5 > > > > > > > > - override > > > > > > Explicit virtual overrides require g++ 4.7: > > > > > > http://gcc.gnu.org/projects/cxx0x.html > > > > you are right, it's also what all other sources referenced in > > http://article.gmane.org/gmane.comp.kde.devel.frameworks/3652 say, no > > idea how that slipped in then, sorry. > > > > > This is trivially to work around by a CMake time check and then just > > > define > > > override to empty. > > > > sure, for KF5 this is not really a problem since Qt provides > > corresponding macros already. I was hoping for real unconditional usage > > though, since Akonadi will need to stay backward-compatible with Qt4 for > > a while longer, and I wanted to avoid to have to basically copy > > qcompilerdetection.h for that. Not to mention that "override" looks much > > nicer than > > "Q_DECL_OVERRIDE" ;) > > I totally agree with that. As I said: detection of this is trivial at CMake > time, maybe I get my C++ feature detection package ready even to be > included in 2.8.12, and if not the stuff can easily be extracted. And Yes, that would be great :-) You still wanted to have a rweview, right ? Alex
Re: Review Request 111576:
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111576/#review36293 --- Ship it! - Lukáš Tinkl On July 18, 2013, 5:58 p.m., Philipp Schmidt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111576/ > --- > > (Updated July 18, 2013, 5:58 p.m.) > > > Review request for kdelibs and Àlex Fiestas. > > > Description > --- > > A simple hack to make media players more distinguishable until a better > solution can be integrated in KF5 as discussed with afiestas at Akademy 2013. > Follows up on his patches that never got integrated. > > Returns the product instead of the description. > > > Diffs > - > > solid/solid/backends/udev/udevdevice.cpp 02f8b2f > > Diff: http://git.reviewboard.kde.org/r/111576/diff/ > > > Testing > --- > > The two MTP devices I own show up with their correct Product name. > > > Thanks, > > Philipp Schmidt > >
Re: Review Request 111626: Fix Amarok crash with Audio CD inserted
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111626/#review36290
---
The problem is that udisks2 no longer associates drives with a block device,
nothing we can circumvent.
- Lukáš Tinkl
On July 21, 2013, 2:28 p.m., Christoph Feck wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111626/
> ---
>
> (Updated July 21, 2013, 2:28 p.m.)
>
>
> Review request for Amarok, kdelibs and Solid.
>
>
> Description
> ---
>
> https://bugs.kde.org/show_bug.cgi?id=314544#c40 says:
>
> "Looking at the backtrace, KFilePlacesItem tries to detect Audio CDs as
> follows:
>
> Solid::Device d = device();
> if (d.isValid()) {
> if (m_access) {
> return QUrl(KUrl(m_access->filePath()));
> } else if (m_disc && (m_disc->availableContent() &
> Solid::OpticalDisc::Audio)!=0) {
> QString device = d.as()->device();
> return QUrl(QString("audiocd:/?device=%1").arg(device));
> }
> }
>
> The crash happens, because d.as returns 0, in other words,
> Audio CDs are no longer treated as block devices.
>
> Failure of said cast can be easily detected, but the question is rather, if
> it's not a Solid bug, how the code needs to be modified to correctly get the
> audiocd:/ URL for the block device."
>
> While I did not yet figure out, how I can get the block device URL for a disc
> in case block is 0, I propose this temporary fix. It should fix the crash,
> but could fail to correctly invoke the audiocd:/ kio with multiple disc
> drives.
>
>
> This addresses bug 314544.
> http://bugs.kde.org/show_bug.cgi?id=314544
>
>
> Diffs
> -
>
> kfile/kfileplacesitem.cpp 5a12486
>
> Diff: http://git.reviewboard.kde.org/r/111626/diff/
>
>
> Testing
> ---
>
> I could not test, my system has no disc drives.
>
>
> Thanks,
>
> Christoph Feck
>
>
KF5 in Release Mode: C++11 and dates
Hello people, As planned, we had the "KF5 in Release Mode" BoF at Akademy. Quite a lot of information on the current status got shared, but I'm not going to focus on that in this email. I'm writing this email to let everyone know about the two main novelties which got discussed during this meeting. First topic is about the C++11 support in KF5. Volker added an email about that, but I thought I'd run through it again. So far, no C++11 feature was used in KF5. The default rule on that topic was "let's do it the same way than Qt" since one of the main goals of KF5 is to make our technologies desirable for Qt application developers. After revisiting how it is done in Qt, it turns out that the situation there is less than clear... We agreed on supporting the following minimal versions for the compilers: gcc 4.5, clang 3.1 and VS2010. That means the new rule is as follow: * We're not supporting the full extent of C++11 in KF5 (although I encourage people to use more higher up in the stack); * We're white listing some of the C++11 features in KF5, namely are now allowed: - auto; - rvalue support (except for "*this"); - lambdas. * If you want to use more than the above: - it must be made optional by the use of #ifdef or the Q_* macros (e.g. Q_DECL_OVERRIDE); - both the binaries built with or without those extra features must be binary compatible. Note I also added the above to the policies for Frameworks: http://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11 Second topic, more to the point of the BoF, is about getting ourselves in release mode. The situation in KF5 is getting clearer by the day and as such we're seeing progress toward meeting a releasable state. This is why we've been already filtering the type of tasks we distribute to focus only on the "must have" tasks. Every other type of tasks are put on a waiting list for post 5.0 work. With this ongoing shift of strategy and mindset, we can play a bit the prediction game. At the current pace and amount of effort put into the project, it is feasible to roll out a technology preview in December 2013 latest with a good confidence level. If the tooling supports it, that preview probably won't be released in one go but in batches (to be confirmed later) in order to find the problems along the way. With such a strategy we might be able to be very close to a final release in March 2014. It's clearly not a given, we'll reevaluate in December if we're still on track, as you know in volunteer projects the amount of available effort can vary greatly (and those estimates assume it will be constant). Still, that makes March 2014 a very worthwhile goal to pursue, so let's try to meet it nonetheless. That's it for now, thanks for your attention. Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part.
