Re: Review Request 124876: KSycoca: check timestamps and run kbuildsycoca if needed. No kded needed anymore.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/#review84583 --- src/sycoca/ksycoca.cpp (line 574) <https://git.reviewboard.kde.org/r/124876/#comment58543> Does the added `(void)` disable some kind of warning? src/sycoca/ksycoca.cpp (line 592) <https://git.reviewboard.kde.org/r/124876/#comment58544> random thought: Is the return value given by kbuildsyscoca useful? - Vishesh Handa On Aug. 28, 2015, 9:41 a.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124876/ > --- > > (Updated Aug. 28, 2015, 9:41 a.m.) > > > Review request for KDE Frameworks, Boudewijn Rempt and Vishesh Handa. > > > Repository: kservice > > > Description > --- > > If 1.5s have passed since the last time we checked, we compare the mtime of > the directories (12 dirs on my system) with the timestamp stored in the > ksycoca database (which indicates that all changes prior to that time > are in the DB). > > Note that we only check the mtime of dirs, not files. Therefore manually > editing an installed .desktop file will require touching the dir, or > running kbuildsycoca5. > > > Diffs > - > > src/sycoca/ksycoca.cpp b7f7abc88db90d784851d91036069e0647fdcbf6 > src/sycoca/ksycoca_p.h 9f403d6f4be2b406f4985f668176cfa56a5898ea > > Diff: https://git.reviewboard.kde.org/r/124876/diff/ > > > Testing > --- > > the kservice unittests still pass > > > Thanks, > > David Faure > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125243: Trivial CMake corrections for Baloo
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125243/#review85460 --- Ship it! Ship It! - Vishesh Handa On Sept. 15, 2015, 5:15 p.m., Hrvoje Senjan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125243/ > --- > > (Updated Sept. 15, 2015, 5:15 p.m.) > > > Review request for KDE Frameworks. > > > Repository: baloo > > > Description > --- > > 1) Be explicit about which modules are required > 2) Find dependancies as the rest of KF5 ecosystem > > > Diffs > - > > CMakeLists.txt d419b91 > KF5BalooConfig.cmake.in cc0f543 > src/file/CMakeLists.txt 0b5d1d9 > > Diff: https://git.reviewboard.kde.org/r/125243/diff/ > > > Testing > --- > > Builds. > > > Thanks, > > Hrvoje Senjan > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120393: [kdelibs4support] Kill dead code
> On May 26, 2015, 8:05 p.m., Hrvoje Senjan wrote: > > Ok. Will update diff to kill nepomuk only. What about forwarding headers? > > They are still installed Ping. What's the status on this? - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120393/#review80866 --- On March 18, 2015, 6:24 p.m., Hrvoje Senjan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120393/ > --- > > (Updated March 18, 2015, 6:24 p.m.) > > > Review request for KDE Frameworks, David Faure and Vishesh Handa. > > > Repository: kdelibs4support > > > Description > --- > > Strigi check has been removed in commit > c8f4c69650c71276b2a2263212addde63764e58b, and soprano wasn't even ported to > Qt5 (afaik), so this was never compiled. > > > Diffs > - > > autotests/kfilemetainfotest.cpp c751cdd > src/CMakeLists.txt b662893 > src/config-kdelibs4support.h.cmake 1af3ee0 > src/kio/kfilemetadataconfigurationwidget.cpp 259b205 > src/kio/kfilemetadataprovider.cpp 3468546 > src/kio/kfilemetadataprovider_p.h 31137b2 > src/kio/kfilemetadatawidget.cpp 1edb069 > src/kio/kfilemetainfo.cpp eae1295 > src/kio/kfilemetainfoitem.cpp 62f760d > src/kio/kfilemetainfoitem_p.h 8929e46 > src/kio/knfotranslator.cpp 8eec6a1 > > Diff: https://git.reviewboard.kde.org/r/120393/diff/ > > > Testing > --- > > > Thanks, > > Hrvoje Senjan > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125762: External extractor plugin support for KFileMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125762/#review87334 --- I still have many thoughts regarding this particular approach, and third party plugins in general. I'm trying to write them into a cohesive blob. src/extractors/externalextractor.cpp (line 69) <https://git.reviewboard.kde.org/r/125762/#comment59973> Why move from the C++ for syntax to Q_FOREACH? There doesn't seem to be any advantage in this case. - Vishesh Handa On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125762/ > --- > > (Updated Oct. 24, 2015, 12:19 p.m.) > > > Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa. > > > Repository: kfilemetadata > > > Description > --- > > This patch introduces support for external metadata extractors in > KFileMetaData > > The external extractors themselves can be written in any language, provided > that it can be executed as a standalone executable (compiled or script with a > hashbang), with command line arguments, and can output data to stdout. > > The extractors are executed like so: > > * `extractor --mimetypes` - outputs a list of mimetypes supported by the > extractor, one per line. > * `extractor filename` - outputs a json document with the metadata. The keys > are such that they can be directly used with PropertyInfo::fromName(). > > At the KFileMetaData end, an additional internal plugin (ExternalExtractor) > is provided that forms a conduit between external extractors and the internal > API. This plugin looks for executables called > kfilemetadata_extractor_ in /usr/bin to find external extractors, > and executes them with the --mimetypes arg to find the list of mimetypes each > extractor supports. ExternalExtractor then claims to support all of these > mimetypes, and then delegates to the extractor executable when doing the > actual extraction. > > > Diffs > - > > README.md 19b1a26 > src/extractors/CMakeLists.txt 5dd223e > src/extractors/externalextractor.h PRE-CREATION > src/extractors/externalextractor.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125762/diff/ > > > Testing > --- > > Tested with the sample executable file extractor (as attched, written in > python) with the dump manual test in KFileMetaData. Works. > > > File Attachments > > > kfilemetadata_extractor_executable > > https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable > > > Thanks, > > Boudhayan Gupta > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126105: Fix yet another crash in Dolphin when Baloo isn't running
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126105/#review88554 --- Ship it! I'm a little hesitant about this, but ship it. A couple of things though - 1. Dolphin should probably not be sending Baloo so many requests if Baloo is disabled. Perhaps we should fix this in Dolphin. 2. This patch is mostly only for the Baloo::File class which causes the db to be opened and closed for each operation. It's expensive, but the main use case that I focussed on was searching. Over there the db is only opened once per application. - Vishesh Handa On Nov. 18, 2015, 7:40 p.m., Boudhayan Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126105/ > --- > > (Updated Nov. 18, 2015, 7:40 p.m.) > > > Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa. > > > Repository: baloo > > > Description > --- > > Add a check in Baloo::Database::open() to return false if we're opening the > database in ReadOnly mode and the database doesn't exist. This fixes a crash > in Dolphin when Baloo isn't running. > > This isn't the entire fix - one will also have to remove > ~/.local/share/baloo/index to not crash anymore; this patch prevents the file > from being recreated everytime Baloo::Database::open() is run, and re-causing > the crash. > > > Diffs > - > > src/engine/database.cpp 4f0579f > > Diff: https://git.reviewboard.kde.org/r/126105/diff/ > > > Testing > --- > > Builds, runs, doesn't crash anymore, the works. > > > Thanks, > > Boudhayan Gupta > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126110: Clean up and armour Baloo::Database::open()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126110/#review88751 --- Ship it! src/engine/database.cpp (line 73) <https://git.reviewboard.kde.org/r/126110/#comment60854> Please remove the comment. It's not longer valid. src/engine/database.cpp (line 192) <https://git.reviewboard.kde.org/r/126110/#comment60855> Perhaps you want to close the env here as well? The assert can fail in production. - Vishesh Handa On Nov. 19, 2015, 11:39 a.m., Boudhayan Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126110/ > --- > > (Updated Nov. 19, 2015, 11:39 a.m.) > > > Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa. > > > Bugs: 353757 > http://bugs.kde.org/show_bug.cgi?id=353757 > > > Repository: baloo > > > Description > --- > > * Add checks for failures > * Add manual checks after Q_ASSERT* (they're not compiled in Release mode) > * Clean up m_env after failure, not just set it to 0 > > > Diffs > - > > src/engine/database.cpp e39eb86 > > Diff: https://git.reviewboard.kde.org/r/126110/diff/ > > > Testing > --- > > Builds, runs, does not crash. > make test succeeds at 100% > > > Thanks, > > Boudhayan Gupta > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125762: External extractor plugin support for KFileMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125762/#review89174 --- If you're planning on pushing this please push it to a testing branch. Untill we actually have some plugins using this we might not be sure of the API. My points below are quite negative and I was hoping on writing a more positive email about the different ways to go about this, but I seem to have been procrastinating and have not written it. Sorry. My main objections with this - 1. One cannot choose between external plugins. Since all of them are in 1-plugin (called ExternalPlugin), and all the mimtypes are just combined. This matters less in the case of Baloo, but when someone else wants to choose specific extractors. It's impossible. 2. Extraction of plain text is simply not supported. 3. No way to differentiate in the implementation of a plugin. We may get plugins for the same mimetype using different technologies, perhaps we need a way to identify which one to choose. Maybe this will never be a problem, but I'm skeptical. 4. Contamination of the PATH, maybe we could put them in a different place which makes it obvious that users should never execute them. 5. Getting the list of extractors now means running a possibly large number of executables with possibly bad implementations. They could just get stuck, and all other plugins will suffer. Perhaps the list of mimetypes supported could be in a desktop file? - Vishesh Handa On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125762/ > --- > > (Updated Oct. 24, 2015, 12:19 p.m.) > > > Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa. > > > Repository: kfilemetadata > > > Description > --- > > This patch introduces support for external metadata extractors in > KFileMetaData > > The external extractors themselves can be written in any language, provided > that it can be executed as a standalone executable (compiled or script with a > hashbang), with command line arguments, and can output data to stdout. > > The extractors are executed like so: > > * `extractor --mimetypes` - outputs a list of mimetypes supported by the > extractor, one per line. > * `extractor filename` - outputs a json document with the metadata. The keys > are such that they can be directly used with PropertyInfo::fromName(). > > At the KFileMetaData end, an additional internal plugin (ExternalExtractor) > is provided that forms a conduit between external extractors and the internal > API. This plugin looks for executables called > kfilemetadata_extractor_ in /usr/bin to find external extractors, > and executes them with the --mimetypes arg to find the list of mimetypes each > extractor supports. ExternalExtractor then claims to support all of these > mimetypes, and then delegates to the extractor executable when doing the > actual extraction. > > > Diffs > - > > README.md 19b1a26 > src/extractors/CMakeLists.txt 5dd223e > src/extractors/externalextractor.h PRE-CREATION > src/extractors/externalextractor.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125762/diff/ > > > Testing > --- > > Tested with the sample executable file extractor (as attched, written in > python) with the dump manual test in KFileMetaData. Works. > > > File Attachments > > > kfilemetadata_extractor_executable > > https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable > > > Thanks, > > Boudhayan Gupta > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127023: [KFileMetadata] Support Origin Email subject/sender/id
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127023/#review92404 --- Fix it, then Ship it! src/propertyinfo.cpp (line 421) <https://git.reviewboard.kde.org/r/127023/#comment63024> I'm not too keen on indexing this unless there is a clear use case. It just increases the size of the index. During the Nepomuk days we used to index and collect way too much information with the hope that we would be find amazing creative uses for the data. It did not end well. - Vishesh Handa On Feb. 9, 2016, 9:09 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127023/ > --- > > (Updated Feb. 9, 2016, 9:09 p.m.) > > > Review request for KDE Frameworks, KDEPIM, Daniel Vrátil, Sebastian Kügler, > and Vishesh Handa. > > > Repository: kfilemetadata > > > Description > --- > > This adds support for the user.xdg.origin.email.subject, > user.xdg.origin.email.sender, user.xdg.origin.email.message-id xattrs [1] to > KFileMetadata. > > This can (should :P) be populated by KMail when you save an attachment. > > Not too happy about the "displayName" I chose. Also we'll need to figure out > what to index and how we can relate this information and present it to the > user in a meaningful way. But at least let's collect the information first > and then we can think about ways to handle this. > > [1] https://wiki.freedesktop.org/www/CommonExtendedAttributes/ > > > Diffs > - > > src/properties.h 6ceaca5 > src/propertyinfo.cpp 4d1fac4 > src/usermetadata.h 9e10d2a > src/usermetadata.cpp 5485d0e > > Diff: https://git.reviewboard.kde.org/r/127023/diff/ > > > Testing > --- > > Not really. Tests pass, though. > > > Thanks, > > Kai Uwe Broulik > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Problems found by the CI system
On Monday, March 24, 2014 06:59:24 PM Ben Cooksley wrote: > > Baloo developers, please take a look at the failure in this log - > http://build.kde.org/view/FAILED/job/baloo_stable/80/console. When > referencing projects outside your own, it is imperative the correct > include statements are used in the CMake logic. > I've pushed a commit. That should hopefully fix it. Could you perhaps add some hook to email me about these failures? We don't have a dedicated mailing list for Baloo and I'm not sure if notifying kde- devel would be a good idea. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Maintainers: Please get ready for release
On Sunday, April 27, 2014 12:32:25 AM Kevin Ottens wrote: > > No maintainer: > - krunner (porting aid) I'm willing to maintain krunner. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118783: Set the normal shortcut when setting the default shortcut as well
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118783/ --- Review request for KDE Frameworks and Martin Gräßlin. Repository: kglobalaccel Description --- When a client calls setDefaultShortcut, only the default shortcut is set. This makes sense, however kglobalaccel doesn't actually do anything with the global shortcut, it just writes it to the configuration and reads it back. All the actual logic is implemented behind "active" or "normal" shortcuts. In kdelibs4, most applications would call KAction::setGlobalShorcut which had a default of setting BOTH the active and the default shortcut. Again, I'm not sure what they point of this was if the default shortcut does not actually do anything. This fixes bugs such as the brightness key not working because Powerdevil only sets the "default" shortcut. Diffs - src/kglobalaccel.cpp 54d18ec Diff: https://git.reviewboard.kde.org/r/118783/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118783: Set the normal shortcut when setting the default shortcut as well
> On June 16, 2014, 7:10 p.m., Martin Gräßlin wrote: > > I would like to get the opinion from people who designed kglobalaccel. > > Personally I do not understand what the default shortcuts are for and why > > they are needed. > > > > From reading the documentation my understanding is that your patch is wrong > > and instead the applications need to be fixed (it's just the default as > > what one can click in the config interface, but not the loaded global > > shortcut). The difference between an "active" and "default" shortcut is that the user cannot configure a default shortcut. Have a look at the systemsettings for global shortcuts. You'll get a better idea. About the application being wrong, I disagree. Most people would think that calling 'setDefaultShortcut(..)' would actually set the shortcut and not just write it to a config file. The way I see it, we either except this patch, or we change kglobalaccel (the daemon) to actually utilize the default shortcut. If you want, I can submit a patch for that. It's just about 4-5 lines. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118783/#review60209 ------- On June 16, 2014, 4:49 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118783/ > --- > > (Updated June 16, 2014, 4:49 p.m.) > > > Review request for KDE Frameworks and Martin Gräßlin. > > > Repository: kglobalaccel > > > Description > --- > > When a client calls setDefaultShortcut, only the default shortcut is > set. This makes sense, however kglobalaccel doesn't actually do anything > with the global shortcut, it just writes it to the configuration and > reads it back. > > All the actual logic is implemented behind "active" or "normal" shortcuts. > In kdelibs4, most applications would call KAction::setGlobalShorcut which > had a default of setting BOTH the active and the default shortcut. Again, > I'm not sure what they point of this was if the default shortcut does not > actually do anything. > > This fixes bugs such as the brightness key not working because > Powerdevil only sets the "default" shortcut. > > > Diffs > - > > src/kglobalaccel.cpp 54d18ec > > Diff: https://git.reviewboard.kde.org/r/118783/diff/ > > > Testing > --- > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118783: Set the normal shortcut when setting the default shortcut as well
> On June 16, 2014, 7:10 p.m., Martin Gräßlin wrote: > > I would like to get the opinion from people who designed kglobalaccel. > > Personally I do not understand what the default shortcuts are for and why > > they are needed. > > > > From reading the documentation my understanding is that your patch is wrong > > and instead the applications need to be fixed (it's just the default as > > what one can click in the config interface, but not the loaded global > > shortcut). > > Vishesh Handa wrote: > The difference between an "active" and "default" shortcut is that the > user cannot configure a default shortcut. Have a look at the systemsettings > for global shortcuts. You'll get a better idea. > > About the application being wrong, I disagree. Most people would think > that calling 'setDefaultShortcut(..)' would actually set the shortcut and not > just write it to a config file. The way I see it, we either except this > patch, or we change kglobalaccel (the daemon) to actually utilize the default > shortcut. If you want, I can submit a patch for that. It's just about 4-5 > lines. > > Martin Gräßlin wrote: > I don't know. Given the documentation I understand it as "this is just > the default shortcut", if one wants to have the global shortcut, the > setShortcut method needs to be used. Whether it's a valid use case to only > have the default shortcut, I'm not sure. But I assume that this was intended > behavior by the people who implemented it. > > Overall my feeling is that default is not intended to be used at all. But > as said: we need input here from people more experienced with kglobalaccel I've been running this patch for a day now, and it is buggy. Custom shortcuts seem to get overwritten with the default one. I'm not bothering to fix it, as it would be best to figure out what course of action we want to take. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118783/#review60209 --- On June 16, 2014, 4:49 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118783/ > --- > > (Updated June 16, 2014, 4:49 p.m.) > > > Review request for KDE Frameworks and Martin Gräßlin. > > > Repository: kglobalaccel > > > Description > --- > > When a client calls setDefaultShortcut, only the default shortcut is > set. This makes sense, however kglobalaccel doesn't actually do anything > with the global shortcut, it just writes it to the configuration and > reads it back. > > All the actual logic is implemented behind "active" or "normal" shortcuts. > In kdelibs4, most applications would call KAction::setGlobalShorcut which > had a default of setting BOTH the active and the default shortcut. Again, > I'm not sure what they point of this was if the default shortcut does not > actually do anything. > > This fixes bugs such as the brightness key not working because > Powerdevil only sets the "default" shortcut. > > > Diffs > - > > src/kglobalaccel.cpp 54d18ec > > Diff: https://git.reviewboard.kde.org/r/118783/diff/ > > > Testing > --- > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
KGlobalAccel Problems
Hey guys I've been looking into kglobalaccel in order to fix some problems that I was having. I seems to have opened a can of worms. Background Information: KGlobalAccel has 2 kinds of shortcuts - Active Shortcuts and Default Shortcuts. Active Shortcuts are the ones currently in use, whereas the default ones are just the ones which an application recommends that should be the default. Setting this default shortcut just results in it being written to a configuration file. In the KDE4 days you would set a global shortcut via KShortcut::setGlobalShortcut(KShortcut, flags). The flags over here were by (Default | Active), so most of the applications would just set it as both the default and the active one. Everything was nice and simple. Now, stuff that has changed - 1. The API has an explicit setDefaultShortcut and setShortcut command. This is bad because setting the deafult shortcut does not set the active one and vice versa. This means applications need to call both. No application currently does this. 2. We allow multiple global shortcuts per application - This is awesome, cause it is super useful, but it also results in - * The KCM only showing one shortcut. And when you change that shortcut, then all the others are cleared. * Having different lists of default and normal shortcuts results in the KCM flaking out and not showing the active shortcut properly. I've been trying to diagnose this, but haven't been successful. I'll try again tomorrow. How do we go about fixing this? Options - a.) Set the active shortcut whenever you set the default shortcut. This is a good compromise, but it also means that any successive call to setShortcut(..) will not do anything cause the shortcut has already been set. Certain application such as KRunner which set one default shortcut, and multiple active shortcuts would break, and need to be fixed. (Which is fine, it's just what happens). We should probably also add some documentation regarding this. b.) Let stuff remain the way it is and add extra documentation. This would mean we would need to update every app using KGlobalAccel and make them set both the default and the active shortcut. c.) Introduce the concept of flags in setShortcut where the default is active + default. This would make it similar to how it was in kdelibs4. The whole active + default thing is still confusing though and we would need better documentation. --- Fixing (2) - I like having multiple default shortcuts available, so I suppose we need to update the KCM? This means fixing up KShortcutEditor from kxmlgui. The code there is a little ambiguous, so I'm not looking forward to that. Also, not sure how it would look visually. Other minor Issues - KXmlGui - KShortcutEditor seems to have some fixmes for KDE5. Do we want to do fix those? Perhaps someone should go through all the frameworks and make these changes or remove the comments? -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118844: Introduce convenient methods to set active and default shortcut
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118844/#review60588
---
Ship it!
I approve.
The only problem I have is with the documentation, but we can improve that in
another patch.
src/kglobalaccel.h
<https://git.reviewboard.kde.org/r/118844/#comment42281>
I think you've typed the letter 'o' instead of zero (0)
src/kglobalaccel.h
<https://git.reviewboard.kde.org/r/118844/#comment42282>
Please add a new line
- Vishesh Handa
On June 20, 2014, 10:01 a.m., Martin Gräßlin wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118844/
> ---
>
> (Updated June 20, 2014, 10:01 a.m.)
>
>
> Review request for KDE Frameworks and Vishesh Handa.
>
>
> Repository: kglobalaccel
>
>
> Description
> ---
>
> Introduce convenient methods to set active and default shortcut
>
> Simplifies the setting of shorctus when default and active should be
> the same or if only one shortcut is needed.
>
>
> Diffs
> -
>
> src/kglobalaccel.h d11881c89e889a77f47c1ba5db42db6ebef665b1
> src/kglobalaccel.cpp 54d18ecf918d4d0890c0a37aec10270119edd103
>
> Diff: https://git.reviewboard.kde.org/r/118844/diff/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Martin Gräßlin
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118783: Set the normal shortcut when setting the default shortcut as well
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118783/ --- (Updated June 20, 2014, 11:18 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks and Martin Gräßlin. Repository: kglobalaccel Description --- When a client calls setDefaultShortcut, only the default shortcut is set. This makes sense, however kglobalaccel doesn't actually do anything with the global shortcut, it just writes it to the configuration and reads it back. All the actual logic is implemented behind "active" or "normal" shortcuts. In kdelibs4, most applications would call KAction::setGlobalShorcut which had a default of setting BOTH the active and the default shortcut. Again, I'm not sure what they point of this was if the default shortcut does not actually do anything. This fixes bugs such as the brightness key not working because Powerdevil only sets the "default" shortcut. Diffs - src/kglobalaccel.cpp 54d18ec Diff: https://git.reviewboard.kde.org/r/118783/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118844: Introduce convenient methods to set active and default shortcut
> On June 20, 2014, 11:17 a.m., Vishesh Handa wrote: > > I approve. > > > > The only problem I have is with the documentation, but we can improve that > > in another patch. > > Martin Gräßlin wrote: > well let's improve directly. What do you think is missing with these new > methods or is that more a general comment? Stuff that should be mentioned - * What a default shortcut is - It is simply something written in a configuration file. This is something the user can later use to reset the shortcut. It cannot be overwritten by a user. Also, maybe how you can change the default shortcut from an application point of view. Maybe even a big disclaimer that this will NOT actually set the shortcut. * What an active shortcut is - It is the shortcut currently in use. This can be changed by the user. I'm not sure if this should be mentioned in this method's documentation or somewhere else. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118844/#review60588 --- On June 20, 2014, 10:01 a.m., Martin Gräßlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118844/ > --- > > (Updated June 20, 2014, 10:01 a.m.) > > > Review request for KDE Frameworks and Vishesh Handa. > > > Repository: kglobalaccel > > > Description > --- > > Introduce convenient methods to set active and default shortcut > > Simplifies the setting of shorctus when default and active should be > the same or if only one shortcut is needed. > > > Diffs > - > > src/kglobalaccel.h d11881c89e889a77f47c1ba5db42db6ebef665b1 > src/kglobalaccel.cpp 54d18ecf918d4d0890c0a37aec10270119edd103 > > Diff: https://git.reviewboard.kde.org/r/118844/diff/ > > > Testing > --- > > > Thanks, > > Martin Gräßlin > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118911: Fix the connection to signal globalShortcutChanged
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118911/#review60964 --- Ship it! Seems correct. - Vishesh Handa On June 24, 2014, 7:15 a.m., Martin Gräßlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118911/ > --- > > (Updated June 24, 2014, 7:15 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kxmlgui > > > Description > --- > > Fix the connection to signal globalShortcutChanged > > KShortcutEditorDelegate was connecting to KAction globalShortcutChanged > signal. This signal does not exist any more. Instead we have > KGlobalAccel::globalShortcutChanged with the QAction and the sequence. > > To fix this we pass the action to the ShortcutEditWidget and connect > there to KGlobalAccel. > > > Diffs > - > > src/kshortcuteditwidget.cpp 4c2f05268c447f0cc358763e1e7490e41971aec5 > src/kshortcutsdialog_p.h 7dd148f4c2b1b64e2b99df5748e09640e7c00734 > src/kshortcutseditordelegate.cpp d974a1ba88d518162598d9eeda282c35788a67ee > > Diff: https://git.reviewboard.kde.org/r/118911/diff/ > > > Testing > --- > > > Thanks, > > Martin Gräßlin > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/ --- Review request for KDE Frameworks. Repository: kinit Description --- Added it in order to debug something. Seemed like a useful things have. Diffs - CMakeLists.txt 631b9d0 tests/CMakeLists.txt PRE-CREATION tests/autostarttest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118960/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118970: libkeduvocdocument: Remove KDELibs4Support dependency.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118970/#review61068 --- The code formatting is quite off. You probably want to run everything under astyle at some point. keduvocdocument/keduvoccsvreader.cpp <https://git.reviewboard.kde.org/r/118970/#comment42529> No fancy headers? :( keduvocdocument/keduvocdocument.cpp <https://git.reviewboard.kde.org/r/118970/#comment42530> I cannot say about KFilterDev, but a TemporaryFile doesn't need to be explicitly closed before destruction. keduvocdocument/sharedkvtmlfiles.cpp <https://git.reviewboard.kde.org/r/118970/#comment42531> Maybe you QDebug instead of QtDebug? The are internally the same thing, but the former looks way more consistent. keduvocdocument/tests/converter.cpp <https://git.reviewboard.kde.org/r/118970/#comment42532> I'm slightly confused. You're using i18n everywhere else, but here you use translate? - Vishesh Handa On June 27, 2014, 4:55 a.m., Jeremy Whiting wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118970/ > --- > > (Updated June 27, 2014, 4:55 a.m.) > > > Review request for KDE Edu, KDE Frameworks, Aleix Pol Gonzalez, and Inge > Wallin. > > > Repository: libkdeedu > > > Description > --- > > Port tests away from KCmdLineArgs to QCommandLineParser. > Port from KDE_DEPRECATED to KEDUVOCDOCUMENT_DEPRECATED (Maybe we should > remove these?) > Port from KLocale to klocalizedstring.h > > > Diffs > - > > CMakeLists.txt 4feb10b67f78530170aaae216b9347cf94c0e51a > keduvocdocument/CMakeLists.txt c242c46fa79f128d63ccdeb5c4f611ad7294c874 > keduvocdocument/keduvocarticle.h b0b69c06d3d91ecc1680ecb18c9dd074beed1cb1 > keduvocdocument/keduvoccsvreader.cpp > e3d471c4ecbb1236bae73714f27e357a1681ec70 > keduvocdocument/keduvoccsvwriter.cpp > 32172e47d3617c082378969735dbd23d3178f06e > keduvocdocument/keduvocdocument.h 9da14c62c84a07ffc53ef46f4c48e35824210d2c > keduvocdocument/keduvocdocument.cpp > 7026ccb9046968821b11ce6e07276d81e58e41c6 > keduvocdocument/keduvockvtml2reader.cpp > 3884724422c318e34a5b8efc5e05481e27f7c6bc > keduvocdocument/keduvockvtmlcompability.cpp > e486729054a9e69fd52c4e4f1031a0bdbb3433cf > keduvocdocument/keduvocpaukerreader.cpp > 4ded3e480222ea9401b8cbfddad2b99ba82036fb > keduvocdocument/keduvoctranslation.h > c41d293755f0df7f87a41300569b093d020beb06 > keduvocdocument/keduvocvokabelnreader.cpp > ffbd64d3cd50e519af319d6d52a1491a7ed1588a > keduvocdocument/keduvocwqlreader.cpp > 0841b1054ec41b93f4fffb1165c2c0d51806aeae > keduvocdocument/keduvocxdxfreader.cpp > 4317f6ff1fbc8a4a19c2bbcc72af2ace295743b6 > keduvocdocument/sharedkvtmlfiles.cpp > 9f74aa2af2f69f25fca8efa1585b8c8ea1127cd9 > keduvocdocument/tests/CMakeLists.txt > ac01a2b4a3e3b925517b2f004feb93bf365344a7 > keduvocdocument/tests/converter.cpp > 1916b2269a88e932beb46f165e6cebebcefecb3c > keduvocdocument/tests/sharedkvtmlfilestest.cpp > 489dad4fcafa0a97fe4e93905206fb0fc53e0cf5 > > Diff: https://git.reviewboard.kde.org/r/118970/diff/ > > > Testing > --- > > It still builds and the tests run. > > Unrelated the sharedkvtmlfilestest exposes a bug in KEduVocDocument::open > where the KFilterDev wont open, (bug from previous commit) not sure what to > do here to fix it though. > > > Thanks, > > Jeremy Whiting > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/#review61258 --- ping? It's just a test. - Vishesh Handa On June 26, 2014, 2:48 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118960/ > --- > > (Updated June 26, 2014, 2:48 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kinit > > > Description > --- > > Added it in order to debug something. Seemed like a useful things have. > > > Diffs > - > > CMakeLists.txt 631b9d0 > tests/CMakeLists.txt PRE-CREATION > tests/autostarttest.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/118960/diff/ > > > Testing > --- > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: qt5 polkit-qt-1 and kdesrc-build
On Sat, Jun 28, 2014 at 5:09 PM, Sune Vuorela wrote: > On 2014-06-27, David Faure wrote: > > > > This question is still open. We're releasing kauth as part of KF5 but > > polkit-qt-1 isn't getting released. > > > > Is there any maintainer for polkit-qt-1? > > > > For that matter, who maintains KAuth, which optionally depends on > polkit-qt-1? > > Isn't KAuth fully useless on linux without polkit-qt ? > Yes. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/ --- (Updated June 30, 2014, 12:45 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kinit Description --- Added it in order to debug something. Seemed like a useful things have. Diffs - CMakeLists.txt 631b9d0 tests/CMakeLists.txt PRE-CREATION tests/autostarttest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118960/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119110: Release Blocker - KProtocolManager: Fix double mutex locking on a non recursive mutex
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119110/ --- Review request for KDE Frameworks and David Faure. Repository: kio Description --- KProtocolManager: Fix double mutex locking on a non recursive mutex Currently called reparseConfiguration results in an infinite block because the code tries to lock the mutex twice even though it is not a recursive mutex. Stack trace - KProtocolManager::entryMap -> Tries to lock it KIO::SlaveConfigPrivate::readGlobalConfig KIO::SlaveConfig::reset KProtocolManager::reparseConfiguration -> Holds the lock This can easily be solved by making the mutex recursive, but that breaks the asserts as they are using QMutex::tryLock to check if the mutex is locked. With the mutex being recursive tryLock just results in them getting locked again. I'm not sure if this is the correct way of fixing it Here is the full backtrace if anyone is interested - #0 0x7328a359 in syscall () from /usr/lib/libc.so.6 #1 0x73e21830 in _q_futex (addr=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>, op=0, val=3, timeout=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:154 #2 0x73e21a04 in lockInternal_helper (d_ptr=..., timeout=-1, elapsedTimer=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:195 #3 0x73e21891 in QBasicMutex::lockInternal (this=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:211 #4 0x73e2164c in QMutex::lock (this=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex.cpp:225 #5 0x72eed989 in QMutexLocker::QMutexLocker (this=0x7fffc6f0, m=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) at /opt/qt5/include/QtCore/qmutex.h:136 #6 0x72ee7f27 in KProtocolManager::entryMap (group=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:294 #7 0x72ee5748 in KIO::SlaveConfigPrivate::readGlobalConfig (this=0x7b5d10) at /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:73 #8 0x72ee5fa6 in KIO::SlaveConfig::reset (this=0x7b6b20) at /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:227 #9 0x72ee7dfc in KProtocolManager::reparseConfiguration () at /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:278 #10 0x72ea37ee in KIO::SessionData::reset (this=0x7b52f0) at /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:136 #11 0x72ea3289 in KIO::SessionData::configDataFor (this=0x7b52f0, configData=..., proto=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:102 #12 0x72edb7f9 in KIO::SchedulerPrivate::metaDataFor (this=0x7b52d0, protocol=..., proxyList=..., url=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1049 #13 0x72edc18b in KIO::SchedulerPrivate::setupSlave (this=0x7b52d0, slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, config=0x0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1094 #14 0x72edb737 in setupSlave (slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, config=0x0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1042 #15 0x72eda759 in KIO::ProtoQueue::startAJob (this=0x7c11b0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:621 #16 0x72edd795 in KIO::ProtoQueue::qt_static_metacall (_o=0x7c11b0, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7fffcfa0) at /home/vishesh/kde5/build/frameworks/kio/src/core/moc_scheduler_p.cpp:244 #17 0x740879bf in QMetaObject::activate (sender=0x7c1208, signalOffset=3, local_signal_index=0, argv=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/kernel/qobject.cpp:3680 Diffs - src/core/kprotocolmanager.cpp 63baa6d Diff: https://git.reviewboard.kde.org/r/119110/diff/ Testing --- This can be tested by running `khotnewstuff "plasmoids.knsrc"` in knewstuff/tests/. Without this patch it just blocks. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119110: Release Blocker - KProtocolManager: Fix double mutex locking on a non recursive mutex
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119110/ --- (Updated July 4, 2014, 9:58 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks and David Faure. Repository: kio Description --- KProtocolManager: Fix double mutex locking on a non recursive mutex Currently called reparseConfiguration results in an infinite block because the code tries to lock the mutex twice even though it is not a recursive mutex. Stack trace - KProtocolManager::entryMap -> Tries to lock it KIO::SlaveConfigPrivate::readGlobalConfig KIO::SlaveConfig::reset KProtocolManager::reparseConfiguration -> Holds the lock This can easily be solved by making the mutex recursive, but that breaks the asserts as they are using QMutex::tryLock to check if the mutex is locked. With the mutex being recursive tryLock just results in them getting locked again. I'm not sure if this is the correct way of fixing it Here is the full backtrace if anyone is interested - #0 0x7328a359 in syscall () from /usr/lib/libc.so.6 #1 0x73e21830 in _q_futex (addr=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>, op=0, val=3, timeout=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:154 #2 0x73e21a04 in lockInternal_helper (d_ptr=..., timeout=-1, elapsedTimer=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:195 #3 0x73e21891 in QBasicMutex::lockInternal (this=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:211 #4 0x73e2164c in QMutex::lock (this=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex.cpp:225 #5 0x72eed989 in QMutexLocker::QMutexLocker (this=0x7fffc6f0, m=0x731a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) at /opt/qt5/include/QtCore/qmutex.h:136 #6 0x72ee7f27 in KProtocolManager::entryMap (group=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:294 #7 0x72ee5748 in KIO::SlaveConfigPrivate::readGlobalConfig (this=0x7b5d10) at /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:73 #8 0x72ee5fa6 in KIO::SlaveConfig::reset (this=0x7b6b20) at /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:227 #9 0x72ee7dfc in KProtocolManager::reparseConfiguration () at /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:278 #10 0x72ea37ee in KIO::SessionData::reset (this=0x7b52f0) at /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:136 #11 0x72ea3289 in KIO::SessionData::configDataFor (this=0x7b52f0, configData=..., proto=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:102 #12 0x72edb7f9 in KIO::SchedulerPrivate::metaDataFor (this=0x7b52d0, protocol=..., proxyList=..., url=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1049 #13 0x72edc18b in KIO::SchedulerPrivate::setupSlave (this=0x7b52d0, slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, config=0x0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1094 #14 0x72edb737 in setupSlave (slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, config=0x0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1042 #15 0x72eda759 in KIO::ProtoQueue::startAJob (this=0x7c11b0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:621 #16 0x72edd795 in KIO::ProtoQueue::qt_static_metacall (_o=0x7c11b0, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7fffcfa0) at /home/vishesh/kde5/build/frameworks/kio/src/core/moc_scheduler_p.cpp:244 #17 0x740879bf in QMetaObject::activate (sender=0x7c1208, signalOffset=3, local_signal_index=0, argv=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/kernel/qobject.cpp:3680 Diffs - src/core/kprotocolmanager.cpp 63baa6d Diff: https://git.reviewboard.kde.org/r/119110/diff/ Testing --- This can be tested by running `khotnewstuff "plasmoids.knsrc"` in knewstuff/tests/. Without this patch it just blocks. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119110: Release Blocker - KProtocolManager: Fix double mutex locking on a non recursive mutex
> On July 4, 2014, 7:22 p.m., David Faure wrote: > > recursive mutexes are more expensive. Can you try this patch instead? > > http://www.davidfaure.fr/2014/kprotocolmanager.cpp.diff > > David Faure wrote: > Actually, nothing beats a unittest :) Added, fix pushed. > > http://commits.kde.org/kio/842bc8008f5eed03698e8ec67351e791b65ad2b1 > > I'll repack kio after dinner. > > David Faure wrote: > Done (v4.100.0-rc2). You can discard this RR. Just out of curiosity, what's so expensive about Recursive mutexes? - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119110/#review61614 ----------- On July 4, 2014, 9:58 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119110/ > --- > > (Updated July 4, 2014, 9:58 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > --- > > KProtocolManager: Fix double mutex locking on a non recursive mutex > > Currently called reparseConfiguration results in an infinite block > because the code tries to lock the mutex twice even though it is not a > recursive mutex. > > Stack trace - > KProtocolManager::entryMap -> Tries to lock it > KIO::SlaveConfigPrivate::readGlobalConfig > KIO::SlaveConfig::reset > KProtocolManager::reparseConfiguration -> Holds the lock > > This can easily be solved by making the mutex recursive, but that breaks > the asserts as they are using QMutex::tryLock to check if the mutex is > locked. With the mutex being recursive tryLock just results in them > getting locked again. > > I'm not sure if this is the correct way of fixing it > > Here is the full backtrace if anyone is interested - > > #0 0x7328a359 in syscall () from /usr/lib/libc.so.6 > #1 0x73e21830 in _q_futex (addr=0x731a4b00 <(anonymous > namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>, op=0, > val=3, timeout=0x0) > at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:154 > #2 0x73e21a04 in lockInternal_helper (d_ptr=..., timeout=-1, > elapsedTimer=0x0) at > /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:195 > #3 0x73e21891 in QBasicMutex::lockInternal (this=0x731a4b00 > <(anonymous > namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) > at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:211 > #4 0x73e2164c in QMutex::lock (this=0x731a4b00 <(anonymous > namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) > at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex.cpp:225 > #5 0x72eed989 in QMutexLocker::QMutexLocker (this=0x7fffc6f0, > m=0x731a4b00 <(anonymous > namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>) > at /opt/qt5/include/QtCore/qmutex.h:136 > #6 0x72ee7f27 in KProtocolManager::entryMap (group=...) at > /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:294 > #7 0x72ee5748 in KIO::SlaveConfigPrivate::readGlobalConfig > (this=0x7b5d10) at > /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:73 > #8 0x72ee5fa6 in KIO::SlaveConfig::reset (this=0x7b6b20) at > /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:227 > #9 0x72ee7dfc in KProtocolManager::reparseConfiguration () at > /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:278 > #10 0x72ea37ee in KIO::SessionData::reset (this=0x7b52f0) at > /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:136 > #11 0x72ea3289 in KIO::SessionData::configDataFor (this=0x7b52f0, > configData=..., proto=...) at > /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:102 > #12 0x72edb7f9 in KIO::SchedulerPrivate::metaDataFor (this=0x7b52d0, > protocol=..., proxyList=..., url=...) > at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1049 > #13 0x72edc18b in KIO::SchedulerPrivate::setupSlave (this=0x7b52d0, > slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, > config=0x0) > at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1094 > #14 0x72edb737 in setupSlave (slave=0x82d3d0
Re: Review Request 119155: Make the desktop containment respect minimum sizes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119155/#review61808 --- containments/desktop/package/contents/ui/AppletAppearance.qml <https://git.reviewboard.kde.org/r/119155/#comment43027> The curly brackets aren't really required. Are we supposed to always have them even in QML code? I know it's a requirement in C++ code. containments/desktop/package/contents/ui/AppletAppearance.qml <https://git.reviewboard.kde.org/r/119155/#comment43024> This function would be called twice. Maybe - property Object minimumSize = computeMinimumSize(); property int minimumWidth = minimumSize.width; property int minimumHeight = minimumSize.height; containments/desktop/package/contents/ui/AppletAppearance.qml <https://git.reviewboard.kde.org/r/119155/#comment43023> containments/desktop/package/contents/ui/AppletAppearance.qml <https://git.reviewboard.kde.org/r/119155/#comment43025> You're returning 0 over here. This will result in a warning in line 264 and 265. containments/desktop/package/contents/ui/main.qml <https://git.reviewboard.kde.org/r/119155/#comment43026> ? - Vishesh Handa On July 7, 2014, 12:51 p.m., Marco Martin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119155/ > --- > > (Updated July 7, 2014, 12:51 p.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: plasma-desktop > > > Description > --- > > make the applets respect the minimum size, specified by applets, with the > following logic: > if there is a preferred representation, enforce its minimum size, > if there is a compact representation, enforce its minimum size. > > note: the minimum size of the full representation is not enforced if it's not > the preferred one, because it must still be possible to switch to the compact > one. > > this would fix bugs such as > https://bugs.kde.org/show_bug.cgi?id=337069 > > > Diffs > - > > containments/desktop/package/contents/ui/AppletAppearance.qml 9149f48 > containments/desktop/package/contents/ui/main.qml a9cf53e > > Diff: https://git.reviewboard.kde.org/r/119155/diff/ > > > Testing > --- > > * even if the property reading is buried down in a function, property binding > still works correctly. > > * if the minimum size grows, it may happen the case of overlapping applets, > thing that was avoideed with care by the new layout engine. This may be a > blocker in shipping it for 5.0 > > > Thanks, > > Marco Martin > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Making KFileMetaData a framework
Hello people I would like to make KFileMetaData a framework. It's a simple library for extracting file metadata and text. It depends on the i18n and KArchive (can be made optional), along with a number of external dependencies which are optional. The code is of decent quality, and we have a certain number of unit tests. It's currently only used by Baloo. I would appreciate it if everyone could review the code once before it gets into frameworks. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Making KFileMetaData a framework
On Tuesday, August 05, 2014 07:58:03 PM David Edmundson wrote: > In general that's some of the tidied code I've seen in a long time. Thanks! > > Comments below. One major, most not. > > > TypeInfo/PropertyInfo/SimpleExtractionResult need a working &operator= > otherwise we shallow copy d. > > I can cause a crash in dump.cpp Fixed > > > Extracting info from a file can take a long time, right now you spawn > a separate execuatble for Baloo; but for other uses (i.e dolphin > getting info on a non-indexed file) you might want to add a helper > API; either a new thread or a service like baloo-file-extractor and a > wrapper round calling it. > > > > ExtractorPluginManager::Private::allExtractors > what's going on with the variable "plugins"? You're always checking if > an empty list contains things. Looks leftover from something. > Fixed > > > In properties.h I would space out the enum so that in the future when > you insert extra ones you can put it alongside the relevant category > without breaking API > > i.e > //Audio > BitRate = 100, > Channels, > Duration, > > // Documents > Author =200, > Title, > ... > > Otherwise it'll be an ungrouped mess in future releases if you ever > have to add another audio property. > Might make sense. Currently adding gaps in the properties would break my tests, but it might be a good idea. I'll look more into it. > - > ExtractionPluginManager possibly shouldn't be a QObject? Fixed > > It /might/ be a good idea to make ExtractionPluginManager static so > you don't load the plugins every time (which can be a bit expensive) > Hmm, Possibly. > > > > dump.cpp should check there's at least one arg. > (I know it's a test, but you explicitly check for >=2 but not <1) > Fixed Thanks for reviewing the code. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Making KFileMetaData a framework
On Tuesday, August 05, 2014 11:19:16 PM Kevin Ottens wrote: > Hello, > > On Tuesday 05 August 2014 18:36:24 Vishesh Handa wrote: > > I would appreciate it if everyone could review the code once before it > > gets > > into frameworks. > > metainfo.yaml should have "release: false" until it's part of a KF release. > Also it should be integration and not functional (relies on plugins). Fixed > > The "test" folder should be named "tests" (see directory structure policy). > Fixed > Public headers should use <> style include. Also it seems you're not > following the k convention but the namespace convention for your classes, > then the includes in public headers should be namespaced as well (e.g. > ). > > I'm not sure why ExtractorPluginManager is exported. A function in a > namespace would be enough, there's no point in instantiating a manager by > hand from the client code perspective, at best looks like leaking an > implementation detail. We could just expose a function which loads all the Extractors, but then the client side would need to iterate over all of them and figure out which ones match the mimetype they are targeting. I was hoping to avoid that. Currently, ExtractorPluginManager just provides a fetchExtractors(mimetype) function. This cannot just be made a function as one doesn't want to load all the plugins each time. Hmm, or maybe we just save all the plugins in a static variable. > > Similarly the ExtractorPlugin naming is odd in the public API as it states > it is necessarily a plugin (implementation detail). I'd rename it > ExtractorInterface, I'd drop the suffix altogether or I'd keep > ExtractorPlugin for plugin implementors while they'd be wrapped in > Extractor instances on the client code side (I think that's actually my > favorite solution). Just to clarify - * ExtractorPlugin - All plugins would derive from this. We could also call it ExtractorInterface (I do like this name better). * We have an Extractor class which is just a wrapper on top of the ExtractorInterface which does ... ? > > AFAICT there's no reason for ExtractorPlugin to inherit from QObject at that > point. Same thing for ExtractorPluginManager. Fixed ExtractorPluginManager ExtractorPlugin - This now requires every plugin to derive from both ExtractorPlugin and QObject. This might be less friendly. > > Creating by hand the ExtractionResult, then passing it by pointer to the > extract method looks odd. I'd expect calling extract() with a bunch of > parameters and getting a result back (another reason for wrapping plugins on > the client side). The idea was that every client should implement their own ExtractionResult. By default it is a pure virtual class. Cause you really don't want to be storing all the text in memory. > > The whole API is synchronous which we probably don't want. It'd be better to > have an async API (much better to build up sync on top of async than the > contrary). Hmm, how would we do async? I thought people could just run it in another thread / process if they want. The only thing I can think of changing is that the plugins return the result later via a signal instead of doing all the work in one function. > > What about the thread safety? At a glance I would say ExtractorPluginManager > is not > > That's about it after a quick glance while tired. Keep us posted for a > second round. Thanks for the review! > > Regards. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Making KFileMetaData a framework
On Wednesday, August 06, 2014 04:09:19 PM Milian Wolff wrote: > > > > Hmm, how would we do async? I thought people could just run it in another > > thread / process if they want. The only thing I can think of changing is > > that the plugins return the result later via a signal instead of doing all > > the work in one function. > > This would be good since it makes it trivial to use it then in a background > Thread - essentially you'd put a worker object into some thread/task which > then emits a signal with the data when its ready. This could be the Manager > (which then automatically finds the correct extractor for the given > mimetype), or a certain Extractor, if you know which one should be used. It might just be easier to make the manager return a QRunnable with all the extractors for that mimetype ready to run. And then you run them in a thread loop. > > Though note that the above does not mean that a call to extract would be > async. It would still be sync. But you could use QFile and its readyRead > signals internally to process stuff asynchronously and then emit the data > once the end of the file is reached. If the signals are there (see above) > this could be done without changing the API and thus could be done later. With Qt we have QFile and readReady signals so this can be done. But a lot of the extractors use third party libraries which are quite often synchronous. > > > What about the thread safety? At a glance I would say > > > ExtractorPluginManager is not > > This should be investigated. Note that it would be fine to load the plugins > in the ctor (which by definition is never thread safe). Just the usage > later to match a given data set/file against the extractors and to get some > result out of that should be made thread safe. > > Bye -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119796: Add a --run-env option
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119796/ --- Review request for KDE Frameworks, Àlex Fiestas, Aleix Pol Gonzalez, and Michael Pyne. Repository: kdesrc-build Description --- This adds a '--run-env' option which spawns a shell with the correct environment so that the user can run the installed programs. This patch does not work as I could not figure out how to get the kdedir and qtdir configuration values from Module.pm. Could someone help? Diffs - modules/ksb/Application.pm 0324002 Diff: https://git.reviewboard.kde.org/r/119796/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: split kdepimlibs
On Tuesday, August 26, 2014 11:50:50 AM Kevin Ottens wrote: > > gpgme++ > > kabc > > kalarmcal > > kblog > > kcalcore > > kcalutils > > This one looks like a dumping ground of random things. Maybe some of it > should move in other frameworks? KAbc definitely doesn't seem like a dumping ground. It's a valuable library that is also used by kpeople. It seems like a good candidate for an individual framework. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 120049: KDirNotify: Use QUrl::toStringList
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120049/ --- Review request for KDE Frameworks and David Faure. Repository: kio Description --- Instead of a custom function. QUrl::toStringList reserves the size of the new list before hand. Diffs - src/core/kdirnotify.cpp 14f4c9c Diff: https://git.reviewboard.kde.org/r/120049/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Initial standalone Plasma::Package repository
On Tue, Sep 16, 2014 at 2:19 PM, Marco Martin wrote: > right now all the classes are still under the Plasma namespace, and should > probably be renamed and cmakes to be cleaned up (especially because it > would > be used by plasma too to the two identically named classes, new and > deprecated > would clash) > Just to confirm - With this, the package class in plasma-framework will now be deprecated? -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 112940: Move all kio tests to the appropriate location
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112940/ --- Review request for KDE Frameworks. Description --- Read title Diffs - kio/tests/CMakeLists.txt 111f137 kio/tests/clipboardupdatertest.h 2cd20ad kio/tests/clipboardupdatertest.cpp d7fcfd4 kio/tests/dataprotocoltest.h 9fee07f kio/tests/dataprotocoltest.cpp d73cc95 kio/tests/fileundomanagertest.h 173df3c kio/tests/fileundomanagertest.cpp dfdeaeb kio/tests/getalltest.cpp a4df4e2 kio/tests/jobguitest.cpp d8d6f73 kio/tests/jobremotetest.h 8667a63 kio/tests/jobremotetest.cpp e7a073e kio/tests/jobtest.h 6142355 kio/tests/jobtest.cpp be8e0d4 kio/tests/kdirlistertest.h 4d3ecc0 kio/tests/kdirlistertest.cpp e6c6f53 kio/tests/kdirlistertest_gui.h 6ae6136 kio/tests/kdirlistertest_gui.cpp 146b60f kio/tests/kdirmodeltest.h 947cdc7 kio/tests/kdirmodeltest.cpp 1ef870d kio/tests/kdirmodeltest_gui.cpp 79ecd65 kio/tests/kfileitemtest.h 1e0cb9a kio/tests/kfileitemtest.cpp f764fbf kio/tests/kioslavetest.h a184a5a kio/tests/kioslavetest.cpp 05e01de kio/tests/kiotesthelper.h 478b895 kio/tests/klocalsocketservertest.h 748fa65 kio/tests/klocalsocketservertest.cpp ba154b4 kio/tests/klocalsockettest.h a76bf9e kio/tests/klocalsockettest.cpp 6eeeff2 kio/tests/kopenwithtest.cpp 16f9082 kio/tests/kprotocolinfotest.cpp b7f640c kio/tests/kurlcompletiontest.cpp 7f9b4f5 kio/tests/kurlrequestertest.cpp ab33eb6 kio/tests/kurlrequestertest_gui.cpp 948085f kio/tests/previewtest.h 7128a85 kio/tests/previewtest.cpp bfc66bf kio/tests/wronglocalsizes.zip 3f76738 staging/kio/autotests/CMakeLists.txt d5f96fa staging/kio/autotests/clipboardupdatertest.h PRE-CREATION staging/kio/autotests/clipboardupdatertest.cpp PRE-CREATION staging/kio/autotests/dataprotocoltest.h PRE-CREATION staging/kio/autotests/dataprotocoltest.cpp PRE-CREATION staging/kio/autotests/fileundomanagertest.h PRE-CREATION staging/kio/autotests/fileundomanagertest.cpp PRE-CREATION staging/kio/autotests/jobguitest.cpp PRE-CREATION staging/kio/autotests/jobremotetest.h PRE-CREATION staging/kio/autotests/jobremotetest.cpp PRE-CREATION staging/kio/autotests/jobtest.h PRE-CREATION staging/kio/autotests/jobtest.cpp PRE-CREATION staging/kio/autotests/kdirlistertest.h PRE-CREATION staging/kio/autotests/kdirlistertest.cpp PRE-CREATION staging/kio/autotests/kdirmodeltest.h PRE-CREATION staging/kio/autotests/kdirmodeltest.cpp PRE-CREATION staging/kio/autotests/kfileitemtest.h PRE-CREATION staging/kio/autotests/kfileitemtest.cpp PRE-CREATION staging/kio/autotests/klocalsocketservertest.h PRE-CREATION staging/kio/autotests/klocalsocketservertest.cpp PRE-CREATION staging/kio/autotests/klocalsockettest.h PRE-CREATION staging/kio/autotests/klocalsockettest.cpp PRE-CREATION staging/kio/autotests/kprotocolinfotest.cpp PRE-CREATION staging/kio/autotests/kurlcompletiontest.cpp PRE-CREATION staging/kio/autotests/kurlrequestertest.cpp PRE-CREATION staging/kio/autotests/wronglocalsizes.zip PRE-CREATION staging/kio/tests/CMakeLists.txt fbda2c6 staging/kio/tests/getalltest.cpp PRE-CREATION staging/kio/tests/kdirlistertest_gui.h PRE-CREATION staging/kio/tests/kdirlistertest_gui.cpp PRE-CREATION staging/kio/tests/kdirmodeltest_gui.cpp PRE-CREATION staging/kio/tests/kioslavetest.h PRE-CREATION staging/kio/tests/kioslavetest.cpp PRE-CREATION staging/kio/tests/kopenwithtest.cpp PRE-CREATION staging/kio/tests/kurlrequestertest_gui.cpp PRE-CREATION staging/kio/tests/previewtest.h PRE-CREATION staging/kio/tests/previewtest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112940/diff/ Testing --- All the tests pass. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 112957: Move KMimeTypeChooser from kio to KWidgetAddons
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112957/ --- Review request for KDE Frameworks. Description --- Removed usage of KConfig for saving the size of the dialog and restoring it when the dialog is created. Removed i18n usage from the test. Changed all char* strings to QLatin1String Changed all char to QLatin1Char Diffs - kio/CMakeLists.txt 25dc6d9 kio/kio/kmimetypechooser.h 9b5a0b7 kio/kio/kmimetypechooser.cpp c68c6e5 kio/tests/CMakeLists.txt 111f137 kio/tests/kmimetypechoosertest_gui.cpp 5c7280e tier1/kwidgetsaddons/src/CMakeLists.txt da55c14 tier1/kwidgetsaddons/src/kmimetypechooser.h PRE-CREATION tier1/kwidgetsaddons/src/kmimetypechooser.cpp PRE-CREATION tier1/kwidgetsaddons/tests/CMakeLists.txt a78ab21 tier1/kwidgetsaddons/tests/kmimetypechoosertest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112957/diff/ Testing --- The kmimetypechooser test works Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112957: Move KMimeTypeChooser from kio to KWidgetAddons
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112957/ --- (Updated Sept. 26, 2013, 11:11 a.m.) Review request for KDE Frameworks. Changes --- Use QStringLiteral instead of QLatin1String Description --- Removed usage of KConfig for saving the size of the dialog and restoring it when the dialog is created. Removed i18n usage from the test. Changed all char* strings to QLatin1String Changed all char to QLatin1Char Diffs (updated) - kio/CMakeLists.txt 25dc6d9 kio/kio/kmimetypechooser.h 9b5a0b7 kio/kio/kmimetypechooser.cpp c68c6e5 kio/tests/CMakeLists.txt 111f137 kio/tests/kmimetypechoosertest_gui.cpp 5c7280e tier1/kwidgetsaddons/src/CMakeLists.txt da55c14 tier1/kwidgetsaddons/src/kmimetypechooser.h PRE-CREATION tier1/kwidgetsaddons/src/kmimetypechooser.cpp PRE-CREATION tier1/kwidgetsaddons/tests/CMakeLists.txt a78ab21 tier1/kwidgetsaddons/tests/kmimetypechoosertest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112957/diff/ Testing --- The kmimetypechooser test works Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112957: Move KMimeTypeChooser from kio to KWidgetAddons
> On Sept. 26, 2013, 11:56 a.m., Stephen Kelly wrote: > > You seem to be doing 3 different things here. Please make sure to do each > > one in a separate commit. Alright. I'll split it into 3 different commits. Do you want 3 different reviews as well? Or will this one do? - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112957/#review40856 --- On Sept. 26, 2013, 11:11 a.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112957/ > --- > > (Updated Sept. 26, 2013, 11:11 a.m.) > > > Review request for KDE Frameworks. > > > Description > --- > > Removed usage of KConfig for saving the size of the dialog and restoring it > when the dialog is created. > > Removed i18n usage from the test. > Changed all char* strings to QLatin1String > Changed all char to QLatin1Char > > > Diffs > - > > kio/CMakeLists.txt 25dc6d9 > kio/kio/kmimetypechooser.h 9b5a0b7 > kio/kio/kmimetypechooser.cpp c68c6e5 > kio/tests/CMakeLists.txt 111f137 > kio/tests/kmimetypechoosertest_gui.cpp 5c7280e > tier1/kwidgetsaddons/src/CMakeLists.txt da55c14 > tier1/kwidgetsaddons/src/kmimetypechooser.h PRE-CREATION > tier1/kwidgetsaddons/src/kmimetypechooser.cpp PRE-CREATION > tier1/kwidgetsaddons/tests/CMakeLists.txt a78ab21 > tier1/kwidgetsaddons/tests/kmimetypechoosertest.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/112957/diff/ > > > Testing > --- > > The kmimetypechooser test works > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112940: Move all kio tests to the appropriate location
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112940/ --- (Updated Sept. 26, 2013, 3:08 p.m.) Review request for KDE Frameworks. Changes --- Use --find-copies-harder while generating the diff Description --- Read title Diffs (updated) - kio/tests/CMakeLists.txt 111f137 kio/tests/clipboardupdatertest.h kio/tests/clipboardupdatertest.cpp kio/tests/dataprotocoltest.h kio/tests/dataprotocoltest.cpp kio/tests/fileundomanagertest.h kio/tests/fileundomanagertest.cpp kio/tests/getalltest.cpp kio/tests/jobguitest.cpp kio/tests/jobremotetest.h kio/tests/jobremotetest.cpp kio/tests/jobtest.h kio/tests/jobtest.cpp kio/tests/kdirlistertest.h kio/tests/kdirlistertest.cpp kio/tests/kdirlistertest_gui.h kio/tests/kdirlistertest_gui.cpp kio/tests/kdirmodeltest.h kio/tests/kdirmodeltest.cpp kio/tests/kdirmodeltest_gui.cpp kio/tests/kfileitemtest.h kio/tests/kfileitemtest.cpp kio/tests/kioslavetest.h kio/tests/kioslavetest.cpp kio/tests/kiotesthelper.h 478b895 kio/tests/klocalsocketservertest.h kio/tests/klocalsocketservertest.cpp kio/tests/klocalsockettest.h kio/tests/klocalsockettest.cpp kio/tests/kopenwithtest.cpp kio/tests/kprotocolinfotest.cpp b7f640c kio/tests/kurlcompletiontest.cpp kio/tests/kurlrequestertest.cpp kio/tests/kurlrequestertest_gui.cpp kio/tests/previewtest.h kio/tests/previewtest.cpp kio/tests/wronglocalsizes.zip staging/kio/autotests/CMakeLists.txt d5f96fa staging/kio/tests/CMakeLists.txt fbda2c6 Diff: http://git.reviewboard.kde.org/r/112940/diff/ Testing --- All the tests pass. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Removing the KFileMetadataWidget
Hello people The current KFileMetaDataWidget uses the Nepomuk libraries which no longer exist in kdelibs. So when we finally start splitting the libraries into their own git repos, kde4support will detect that Nepomuk is not installed and that will make KFileMetaDataWidget useless - It literally will not do anything. I'm planning on removing KFileMetaDataWidget completely, and marking it as deprecated in kdelibs4. Applications which still use it (Konversation and KGet) can start using Nepomuk::FileMetaDataWidget which offers the same api. Objections? -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113077: Get Kross ready for tier3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113077/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- This patch makes kross ready for moving to tier3. The changes were done in a separate branch (which I can push if required). The git log looks like this - 783b0a3 Kross: Remove module name from the header includes 8b7ac05 Kross: Do not set EXECUTABLE_OUTPUT_PATH 6c62ace Kross: Add feature_summary d9b63be Kross: Install the plugins in the QT_PLULGIN_INSTALL_DIR 2c627df Kross: Add CMake stuff along with KrossConfig.cmake a28624b Kross: Follow frameworks directory scheme 741b86c KRoss: Camel case the library names Kross does not have any unit tests, and some of its normal tests seem to be segfaulting as well. I'm not sure what to do about that. Also when Kross is being split, its other plugins will needs to be added as well. Kdelibs only contains the kjs and QtScript plugins. And finally, the QtScript plugin has a test - Should that be moved to the tests folder? Diffs - kross/CMakeLists.txt 3424cb8 kross/KrossConfig.cmake.in PRE-CREATION kross/console/CMakeLists.txt da73a6f kross/console/main.cpp kross/core/CMakeLists.txt 5a8b845 kross/core/action.h 3f4d985 kross/core/action.cpp 5c6ee2c kross/core/actioncollection.h 9e90df6 kross/core/actioncollection.cpp 89d8282 kross/core/childreninterface.h 0b11b2e kross/core/errorinterface.h bb23235 kross/core/interpreter.h 95293e4 kross/core/interpreter.cpp kross/core/krossconfig.h 481c4b6 kross/core/krossconfig.cpp kross/core/manager.h aefdf87 kross/core/manager.cpp e0dddf1 kross/core/metafunction.h 1413289 kross/core/metatype.h 08f7c6c kross/core/object.h 6f2b6a4 kross/core/object.cpp kross/core/script.h 1701883 kross/core/script.cpp kross/core/wrapperinterface.h kross/kjs/CMakeLists.txt 94af9ae kross/kjs/kjsinterpreter.h kross/kjs/kjsinterpreter.cpp kross/kjs/kjsscript.h kross/kjs/kjsscript.cpp kross/modules/CMakeLists.txt 7391bb0 kross/modules/form.h 2da62c7 kross/modules/form.cpp 862b71b kross/modules/translation.h kross/modules/translation.cpp kross/qts/CMakeLists.txt b85c1c7 kross/qts/interpreter.h kross/qts/interpreter.cpp kross/qts/main.cpp kross/qts/plugin.h kross/qts/plugin.cpp kross/qts/script.h kross/qts/script.cpp kross/qts/test.es kross/qts/values_p.h kross/src/CMakeLists.txt PRE-CREATION kross/src/modules/CMakeLists.txt PRE-CREATION kross/test/CMakeLists.txt c931ec4 kross/test/main.cpp da926fb kross/test/profile.py kross/test/testguiform.py kross/test/testguiform.rb kross/test/testguiform.ui kross/test/testguiqt.py kross/test/testguiqt.rb kross/test/testguitk.py kross/test/testkross.js kross/test/testkross.py kross/test/testobject.h 5383073 kross/test/testobject.cpp 560d88c kross/test/unittest.es kross/test/unittest.js kross/test/unittest.py kross/test/unittest.rb kross/ui/CMakeLists.txt 14ba2d8 kross/ui/model.h 314ff24 kross/ui/model.cpp 4849149 kross/ui/plugin.h kross/ui/plugin.cpp kross/ui/view.h 4241aad kross/ui/view.cpp Diff: http://git.reviewboard.kde.org/r/113077/diff/ Testing --- Some tests segfault. Some of them run. I should probably convert them into autotests. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Failing Tests in Kross
Hey Sebastian I had taken up the task of moving Kross to tier3, and I noticed that most tests (11/12) are failing. The following tests FAILED: 1 - kross-guiform-py (Failed) 2 - kross-guiform-rb (Failed) 3 - kross-guiqt-py (Failed) 4 - kross-guiqt-rb (Failed) 5 - kross-guitk-py (Failed) 7 - kross-test-py (Failed) 8 - kross-unittest-es (SEGFAULT) 9 - kross-unittest-js (SEGFAULT) 10 - kross-unittest-py (Failed) 11 - kross-unittest-rb (Failed) The test in the qts plugin also segfaults. The python and ruby tests fail because of the missing kross plugin. However the js/es ones seem to segault. Do you think you could have a look at them? I'm not comfortable moving it to tier3 with failing tests. There also seems to be a .ui file in the tests directory, which I'm not sure how to run. When Kross moves to its own git repo we should probably move the ruby and python plugin in it as well. Also, can I add a MAINTAINERS file with your name in it? -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113154: Remove KFileMetaDataWidget and friends
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113154/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- Remove KFileMetaDataWidget and friends These have been deprecated in KDE4.[1] This also removes the KFileMetaPropsPlugin in the KPropertiesDialog - The code was commented out so it doesn't really make a difference. Eventually we will need a proper plugin based system so that the Nepomuk2::FileMetadataWidget can be used in the KPropertiesDialog [1] https://git.reviewboard.kde.org/r/113153/ Diffs - staging/kde4support/src/CMakeLists.txt 5eb649c staging/kde4support/src/kio/kcommentwidget.cpp 6223a0c staging/kde4support/src/kio/kcommentwidget_p.h 7a9c710 staging/kde4support/src/kio/kfilemetadataconfigurationwidget.h 52735ad staging/kde4support/src/kio/kfilemetadataconfigurationwidget.cpp 018d183 staging/kde4support/src/kio/kfilemetadataprovider.cpp 59cb238 staging/kde4support/src/kio/kfilemetadataprovider_p.h 0969f53 staging/kde4support/src/kio/kfilemetadatareader.cpp 6a7909c staging/kde4support/src/kio/kfilemetadatareader_p.h af054c2 staging/kde4support/src/kio/kfilemetadatareaderprocess.cpp 0d2b993 staging/kde4support/src/kio/kfilemetadatawidget.h 31dd3c7 staging/kde4support/src/kio/kfilemetadatawidget.cpp 2df2312 staging/kde4support/src/kio/kmetaprops.h b03dd4c staging/kde4support/src/kio/kmetaprops.cpp 46624c5 staging/kde4support/src/kio/knfotranslator.cpp 0494679 staging/kde4support/src/kio/knfotranslator_p.h ddbe4a1 staging/kio/src/widgets/kpropertiesdialog.cpp 63e4435 Diff: http://git.reviewboard.kde.org/r/113154/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113153: Deprecate KFileMetaDataWidget and KFileMetaDataConfigurationWidget
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113153/ --- Review request for KDE Frameworks and kdelibs. Repository: kdelibs Description --- They will no longer exist in KF5, and everyone should be using the Nepomuk widgets instead. Diffs - kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d kio/kfile/kfilemetadatawidget.h 50ddce9 Diff: http://git.reviewboard.kde.org/r/113153/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113157: Remove Nepomuk dependency from kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113157/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- The only class using it is KFileMetaInfoItem which is just using the ontology parts in order to get a better name for a property. It can rely on strigi instead. Diffs - staging/kde4support/src/CMakeLists.txt 5eb649c staging/kde4support/src/config-kde4support.h.cmake 95a092f staging/kde4support/src/kdebug.areas 2cabd98 staging/kde4support/src/kio/kfilemetainfoitem.cpp 9df2602 staging/kde4support/src/kio/kfilemetainfoitem_p.h 959fbd6 Diff: http://git.reviewboard.kde.org/r/113157/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113077: Get Kross ready for tier3
> On Oct. 7, 2013, 2:38 p.m., Martin Gräßlin wrote: > > You broke the build (and forgot to include the REVIEW tag ;-): > > [ 41%] Building CXX object > > kross/src/core/CMakeFiles/KrossCore.dir/krossconfig.cpp.o > > In file included from > > /home/martin/kf5/src/kdelibs-frameworks/kross/src/core/krossconfig.cpp:20:0: > > /home/martin/kf5/src/kdelibs-frameworks/kross/src/core/krossconfig.h:23:41: > > fatal error: kross/core/krosscore_export.h: No such file or directory > > #include > > ^ > > compilation terminated. > > make[2]: *** [kross/src/core/CMakeFiles/KrossCore.dir/krossconfig.cpp.o] > > Error 1 > > make[1]: *** [kross/src/core/CMakeFiles/KrossCore.dir/all] Error 2 > > Martin Gräßlin wrote: > seems like Kevin already fixed it Urgh. I don't understand. I compiled it with a fresh build directory. Something like this should have been caught. - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113077/#review41350 ------- On Oct. 3, 2013, 4:09 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113077/ > --- > > (Updated Oct. 3, 2013, 4:09 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > --- > > This patch makes kross ready for moving to tier3. The changes were done in a > separate branch (which I can push if required). The git log looks like this - > > 783b0a3 Kross: Remove module name from the header includes > 8b7ac05 Kross: Do not set EXECUTABLE_OUTPUT_PATH > 6c62ace Kross: Add feature_summary > d9b63be Kross: Install the plugins in the QT_PLULGIN_INSTALL_DIR > 2c627df Kross: Add CMake stuff along with KrossConfig.cmake > a28624b Kross: Follow frameworks directory scheme > 741b86c KRoss: Camel case the library names > > Kross does not have any unit tests, and some of its normal tests seem to be > segfaulting as well. I'm not sure what to do about that. Also when Kross is > being split, its other plugins will needs to be added as well. Kdelibs only > contains the kjs and QtScript plugins. > > And finally, the QtScript plugin has a test - Should that be moved to the > tests folder? > > > Diffs > - > > kross/CMakeLists.txt 3424cb8 > kross/KrossConfig.cmake.in PRE-CREATION > kross/console/CMakeLists.txt da73a6f > kross/console/main.cpp > kross/core/CMakeLists.txt 5a8b845 > kross/core/action.h 3f4d985 > kross/core/action.cpp 5c6ee2c > kross/core/actioncollection.h 9e90df6 > kross/core/actioncollection.cpp 89d8282 > kross/core/childreninterface.h 0b11b2e > kross/core/errorinterface.h bb23235 > kross/core/interpreter.h 95293e4 > kross/core/interpreter.cpp > kross/core/krossconfig.h 481c4b6 > kross/core/krossconfig.cpp > kross/core/manager.h aefdf87 > kross/core/manager.cpp e0dddf1 > kross/core/metafunction.h 1413289 > kross/core/metatype.h 08f7c6c > kross/core/object.h 6f2b6a4 > kross/core/object.cpp > kross/core/script.h 1701883 > kross/core/script.cpp > kross/core/wrapperinterface.h > kross/kjs/CMakeLists.txt 94af9ae > kross/kjs/kjsinterpreter.h > kross/kjs/kjsinterpreter.cpp > kross/kjs/kjsscript.h > kross/kjs/kjsscript.cpp > kross/modules/CMakeLists.txt 7391bb0 > kross/modules/form.h 2da62c7 > kross/modules/form.cpp 862b71b > kross/modules/translation.h > kross/modules/translation.cpp > kross/qts/CMakeLists.txt b85c1c7 > kross/qts/interpreter.h > kross/qts/interpreter.cpp > kross/qts/main.cpp > kross/qts/plugin.h > kross/qts/plugin.cpp > kross/qts/script.h > kross/qts/script.cpp > kross/qts/test.es > kross/qts/values_p.h > kross/src/CMakeLists.txt PRE-CREATION > kross/src/modules/CMakeLists.txt PRE-CREATION > kross/test/CMakeLists.txt c931ec4 > kross/test/main.cpp da926fb > kross/test/profile.py > kross/test/testguiform.py > kross/test/testguiform.rb > kross/test/testguiform.ui > kross/test/testguiqt.py > kross/test/testguiqt.rb > kross/test/testguitk.py > kross/test/testkross.js > kross/test/testkross.py > kross/test/testobject.h 5383073 > kross/test/testobject.cpp 560d88c > kross/test/unittest.es > kross/test/uni
Re: Review Request 113077: Get Kross ready for tier3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113077/ --- (Updated Oct. 8, 2013, 7:47 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- This patch makes kross ready for moving to tier3. The changes were done in a separate branch (which I can push if required). The git log looks like this - 783b0a3 Kross: Remove module name from the header includes 8b7ac05 Kross: Do not set EXECUTABLE_OUTPUT_PATH 6c62ace Kross: Add feature_summary d9b63be Kross: Install the plugins in the QT_PLULGIN_INSTALL_DIR 2c627df Kross: Add CMake stuff along with KrossConfig.cmake a28624b Kross: Follow frameworks directory scheme 741b86c KRoss: Camel case the library names Kross does not have any unit tests, and some of its normal tests seem to be segfaulting as well. I'm not sure what to do about that. Also when Kross is being split, its other plugins will needs to be added as well. Kdelibs only contains the kjs and QtScript plugins. And finally, the QtScript plugin has a test - Should that be moved to the tests folder? Diffs - kross/CMakeLists.txt 3424cb8 kross/KrossConfig.cmake.in PRE-CREATION kross/console/CMakeLists.txt da73a6f kross/console/main.cpp kross/core/CMakeLists.txt 5a8b845 kross/core/action.h 3f4d985 kross/core/action.cpp 5c6ee2c kross/core/actioncollection.h 9e90df6 kross/core/actioncollection.cpp 89d8282 kross/core/childreninterface.h 0b11b2e kross/core/errorinterface.h bb23235 kross/core/interpreter.h 95293e4 kross/core/interpreter.cpp kross/core/krossconfig.h 481c4b6 kross/core/krossconfig.cpp kross/core/manager.h aefdf87 kross/core/manager.cpp e0dddf1 kross/core/metafunction.h 1413289 kross/core/metatype.h 08f7c6c kross/core/object.h 6f2b6a4 kross/core/object.cpp kross/core/script.h 1701883 kross/core/script.cpp kross/core/wrapperinterface.h kross/kjs/CMakeLists.txt 94af9ae kross/kjs/kjsinterpreter.h kross/kjs/kjsinterpreter.cpp kross/kjs/kjsscript.h kross/kjs/kjsscript.cpp kross/modules/CMakeLists.txt 7391bb0 kross/modules/form.h 2da62c7 kross/modules/form.cpp 862b71b kross/modules/translation.h kross/modules/translation.cpp kross/qts/CMakeLists.txt b85c1c7 kross/qts/interpreter.h kross/qts/interpreter.cpp kross/qts/main.cpp kross/qts/plugin.h kross/qts/plugin.cpp kross/qts/script.h kross/qts/script.cpp kross/qts/test.es kross/qts/values_p.h kross/src/CMakeLists.txt PRE-CREATION kross/src/modules/CMakeLists.txt PRE-CREATION kross/test/CMakeLists.txt c931ec4 kross/test/main.cpp da926fb kross/test/profile.py kross/test/testguiform.py kross/test/testguiform.rb kross/test/testguiform.ui kross/test/testguiqt.py kross/test/testguiqt.rb kross/test/testguitk.py kross/test/testkross.js kross/test/testkross.py kross/test/testobject.h 5383073 kross/test/testobject.cpp 560d88c kross/test/unittest.es kross/test/unittest.js kross/test/unittest.py kross/test/unittest.rb kross/ui/CMakeLists.txt 14ba2d8 kross/ui/model.h 314ff24 kross/ui/model.cpp 4849149 kross/ui/plugin.h kross/ui/plugin.cpp kross/ui/view.h 4241aad kross/ui/view.cpp Diff: http://git.reviewboard.kde.org/r/113077/diff/ Testing --- Some tests segfault. Some of them run. I should probably convert them into autotests. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113154: Remove KFileMetaDataWidget and friends
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113154/ --- (Updated Oct. 10, 2013, 12:56 p.m.) Review request for KDE Frameworks. Changes --- Added a note in KDE5Porting.html Repository: kdelibs Description --- Remove KFileMetaDataWidget and friends These have been deprecated in KDE4.[1] This also removes the KFileMetaPropsPlugin in the KPropertiesDialog - The code was commented out so it doesn't really make a difference. Eventually we will need a proper plugin based system so that the Nepomuk2::FileMetadataWidget can be used in the KPropertiesDialog [1] https://git.reviewboard.kde.org/r/113153/ Diffs (updated) - KDE5PORTING.html 3171afc kdewidgets/kde.widgets b138d4e staging/kde4support/src/CMakeLists.txt 5eb649c staging/kde4support/src/kio/kcommentwidget.cpp 6223a0c staging/kde4support/src/kio/kcommentwidget_p.h 7a9c710 staging/kde4support/src/kio/kfilemetadataconfigurationwidget.h 52735ad staging/kde4support/src/kio/kfilemetadataconfigurationwidget.cpp 018d183 staging/kde4support/src/kio/kfilemetadataprovider.cpp 59cb238 staging/kde4support/src/kio/kfilemetadataprovider_p.h 0969f53 staging/kde4support/src/kio/kfilemetadatareader.cpp 6a7909c staging/kde4support/src/kio/kfilemetadatareader_p.h af054c2 staging/kde4support/src/kio/kfilemetadatareaderprocess.cpp 0d2b993 staging/kde4support/src/kio/kfilemetadatawidget.h 31dd3c7 staging/kde4support/src/kio/kfilemetadatawidget.cpp 2df2312 staging/kde4support/src/kio/kmetaprops.h b03dd4c staging/kde4support/src/kio/kmetaprops.cpp 46624c5 staging/kde4support/src/kio/knfotranslator.cpp 0494679 staging/kde4support/src/kio/knfotranslator_p.h ddbe4a1 staging/kio/src/widgets/kpropertiesdialog.cpp 63e4435 Diff: http://git.reviewboard.kde.org/r/113154/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113154: Remove KFileMetaDataWidget and friends
> On Oct. 14, 2013, 7:31 a.m., Kevin Ottens wrote: > > -1 > > > > I disagree with the removal, OK they get deprecated in KDE4... but it's > > been done only recently (the patch isn't even in yet). We still have a > > couple of users for those classes and it would be one more breakage on our > > SC promise (and one we can avoid at that). > > Kevin Ottens wrote: > Of course I meant for the removals in kde4support. The comments cleanup > in kio I'm fine with it. Well, avoiding that would be mean that I need to either (1) port it to Nepomuk2 and thus get a dependency to nepomuk-core or (2) remove all the Nepomuk code. If it is really required I can go with (2), though it'll be a lot more work. The nepomuk-core replacement classes are almost source compatible with the kio ones. So the port is mostly just changing the class name, and linking to the new library. Also, Konversation and Conquire (Nepomuk app) seems to be the only users of this class. KGet has been ported. Do you still want me to go with (2)? - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113154/#review41676 ------- On Oct. 10, 2013, 12:56 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113154/ > --- > > (Updated Oct. 10, 2013, 12:56 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > --- > > Remove KFileMetaDataWidget and friends > > These have been deprecated in KDE4.[1] This also removes the > KFileMetaPropsPlugin in the KPropertiesDialog - The code was commented > out so it doesn't really make a difference. > > Eventually we will need a proper plugin based system so that the > Nepomuk2::FileMetadataWidget can be used in the KPropertiesDialog > > [1] https://git.reviewboard.kde.org/r/113153/ > > > Diffs > - > > KDE5PORTING.html 3171afc > kdewidgets/kde.widgets b138d4e > staging/kde4support/src/CMakeLists.txt 5eb649c > staging/kde4support/src/kio/kcommentwidget.cpp 6223a0c > staging/kde4support/src/kio/kcommentwidget_p.h 7a9c710 > staging/kde4support/src/kio/kfilemetadataconfigurationwidget.h 52735ad > staging/kde4support/src/kio/kfilemetadataconfigurationwidget.cpp 018d183 > staging/kde4support/src/kio/kfilemetadataprovider.cpp 59cb238 > staging/kde4support/src/kio/kfilemetadataprovider_p.h 0969f53 > staging/kde4support/src/kio/kfilemetadatareader.cpp 6a7909c > staging/kde4support/src/kio/kfilemetadatareader_p.h af054c2 > staging/kde4support/src/kio/kfilemetadatareaderprocess.cpp 0d2b993 > staging/kde4support/src/kio/kfilemetadatawidget.h 31dd3c7 > staging/kde4support/src/kio/kfilemetadatawidget.cpp 2df2312 > staging/kde4support/src/kio/kmetaprops.h b03dd4c > staging/kde4support/src/kio/kmetaprops.cpp 46624c5 > staging/kde4support/src/kio/knfotranslator.cpp 0494679 > staging/kde4support/src/kio/knfotranslator_p.h ddbe4a1 > staging/kio/src/widgets/kpropertiesdialog.cpp 63e4435 > > Diff: http://git.reviewboard.kde.org/r/113154/diff/ > > > Testing > --- > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113154: Remove KFileMetaDataWidget and friends
> On Oct. 14, 2013, 7:31 a.m., Kevin Ottens wrote: > > -1 > > > > I disagree with the removal, OK they get deprecated in KDE4... but it's > > been done only recently (the patch isn't even in yet). We still have a > > couple of users for those classes and it would be one more breakage on our > > SC promise (and one we can avoid at that). > > Kevin Ottens wrote: > Of course I meant for the removals in kde4support. The comments cleanup > in kio I'm fine with it. > > Vishesh Handa wrote: > Well, avoiding that would be mean that I need to either (1) port it to > Nepomuk2 and thus get a dependency to nepomuk-core or (2) remove all the > Nepomuk code. If it is really required I can go with (2), though it'll be a > lot more work. > > The nepomuk-core replacement classes are almost source compatible with > the kio ones. So the port is mostly just changing the class name, and linking > to the new library. Also, Konversation and Conquire (Nepomuk app) seems to be > the only users of this class. KGet has been ported. > > Do you still want me to go with (2)? > > Kevin Ottens wrote: > OK, I guess I miss a piece of information then: What happened to the > Nepomuk API it currently uses? Did it simply disappear? if yes it means even > more broken promises. :-) The Nepomuk API that was originally in kdelibs/nepomuk was removed very long ago. The code now lives in nepomuk-core and nepomuk-widgets. We almost maintain source compatibility (minus some small things). - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113154/#review41676 --- On Oct. 10, 2013, 12:56 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113154/ > --- > > (Updated Oct. 10, 2013, 12:56 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > --- > > Remove KFileMetaDataWidget and friends > > These have been deprecated in KDE4.[1] This also removes the > KFileMetaPropsPlugin in the KPropertiesDialog - The code was commented > out so it doesn't really make a difference. > > Eventually we will need a proper plugin based system so that the > Nepomuk2::FileMetadataWidget can be used in the KPropertiesDialog > > [1] https://git.reviewboard.kde.org/r/113153/ > > > Diffs > - > > KDE5PORTING.html 3171afc > kdewidgets/kde.widgets b138d4e > staging/kde4support/src/CMakeLists.txt 5eb649c > staging/kde4support/src/kio/kcommentwidget.cpp 6223a0c > staging/kde4support/src/kio/kcommentwidget_p.h 7a9c710 > staging/kde4support/src/kio/kfilemetadataconfigurationwidget.h 52735ad > staging/kde4support/src/kio/kfilemetadataconfigurationwidget.cpp 018d183 > staging/kde4support/src/kio/kfilemetadataprovider.cpp 59cb238 > staging/kde4support/src/kio/kfilemetadataprovider_p.h 0969f53 > staging/kde4support/src/kio/kfilemetadatareader.cpp 6a7909c > staging/kde4support/src/kio/kfilemetadatareader_p.h af054c2 > staging/kde4support/src/kio/kfilemetadatareaderprocess.cpp 0d2b993 > staging/kde4support/src/kio/kfilemetadatawidget.h 31dd3c7 > staging/kde4support/src/kio/kfilemetadatawidget.cpp 2df2312 > staging/kde4support/src/kio/kmetaprops.h b03dd4c > staging/kde4support/src/kio/kmetaprops.cpp 46624c5 > staging/kde4support/src/kio/knfotranslator.cpp 0494679 > staging/kde4support/src/kio/knfotranslator_p.h ddbe4a1 > staging/kio/src/widgets/kpropertiesdialog.cpp 63e4435 > > Diff: http://git.reviewboard.kde.org/r/113154/diff/ > > > Testing > --- > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Failing Tests in Kross
Minor update - Sebastian Sauer's email does not seem to be working. I've tried contacting him via twitter. Lets see. If anyone knows how to contact him, please inform me. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113154: Remove KFileMetaDataWidget and friends
> On Oct. 14, 2013, 7:31 a.m., Kevin Ottens wrote: > > -1 > > > > I disagree with the removal, OK they get deprecated in KDE4... but it's > > been done only recently (the patch isn't even in yet). We still have a > > couple of users for those classes and it would be one more breakage on our > > SC promise (and one we can avoid at that). > > Kevin Ottens wrote: > Of course I meant for the removals in kde4support. The comments cleanup > in kio I'm fine with it. > > Vishesh Handa wrote: > Well, avoiding that would be mean that I need to either (1) port it to > Nepomuk2 and thus get a dependency to nepomuk-core or (2) remove all the > Nepomuk code. If it is really required I can go with (2), though it'll be a > lot more work. > > The nepomuk-core replacement classes are almost source compatible with > the kio ones. So the port is mostly just changing the class name, and linking > to the new library. Also, Konversation and Conquire (Nepomuk app) seems to be > the only users of this class. KGet has been ported. > > Do you still want me to go with (2)? > > Kevin Ottens wrote: > OK, I guess I miss a piece of information then: What happened to the > Nepomuk API it currently uses? Did it simply disappear? if yes it means even > more broken promises. :-) > > Vishesh Handa wrote: > The Nepomuk API that was originally in kdelibs/nepomuk was removed very > long ago. The code now lives in nepomuk-core and nepomuk-widgets. We almost > maintain source compatibility (minus some small things). > > Kevin Ottens wrote: > OK, so that's what I had in mind. It means the API used by those classes > in kde4support you're trying to remove is still there for their consumption, > so why not just let them alone? I don't see the benefit of removing them or > porting them to something different, it'll just create more build errors to > code built against kf5, making ports harder. Alright. I'll let this be and start working on nepomuk-core frameworks port. Please note that this means kdesupport will depend on nepomuk-core. It also means that this patch is now invalid - https://git.reviewboard.kde.org/r/113157/ - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113154/#review41676 --- On Oct. 10, 2013, 12:56 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113154/ > --- > > (Updated Oct. 10, 2013, 12:56 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > --- > > Remove KFileMetaDataWidget and friends > > These have been deprecated in KDE4.[1] This also removes the > KFileMetaPropsPlugin in the KPropertiesDialog - The code was commented > out so it doesn't really make a difference. > > Eventually we will need a proper plugin based system so that the > Nepomuk2::FileMetadataWidget can be used in the KPropertiesDialog > > [1] https://git.reviewboard.kde.org/r/113153/ > > > Diffs > - > > KDE5PORTING.html 3171afc > kdewidgets/kde.widgets b138d4e > staging/kde4support/src/CMakeLists.txt 5eb649c > staging/kde4support/src/kio/kcommentwidget.cpp 6223a0c > staging/kde4support/src/kio/kcommentwidget_p.h 7a9c710 > staging/kde4support/src/kio/kfilemetadataconfigurationwidget.h 52735ad > staging/kde4support/src/kio/kfilemetadataconfigurationwidget.cpp 018d183 > staging/kde4support/src/kio/kfilemetadataprovider.cpp 59cb238 > staging/kde4support/src/kio/kfilemetadataprovider_p.h 0969f53 > staging/kde4support/src/kio/kfilemetadatareader.cpp 6a7909c > staging/kde4support/src/kio/kfilemetadatareader_p.h af054c2 > staging/kde4support/src/kio/kfilemetadatareaderprocess.cpp 0d2b993 > staging/kde4support/src/kio/kfilemetadatawidget.h 31dd3c7 > staging/kde4support/src/kio/kfilemetadatawidget.cpp 2df2312 > staging/kde4support/src/kio/kmetaprops.h b03dd4c > staging/kde4support/src/kio/kmetaprops.cpp 46624c5 > staging/kde4support/src/kio/knfotranslator.cpp 0494679 > staging/kde4support/src/kio/knfotranslator_p.h ddbe4a1 > staging/kio/src/widgets/kpropertiesdialog.cpp 63e4435 > > Diff: http://git.reviewboard.kde.org/r/113154/diff/ > > > Testing > --- > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113153: Deprecate KFileMetaDataWidget and KFileMetaDataConfigurationWidget
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113153/ --- (Updated Oct. 14, 2013, 4:16 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and kdelibs. Repository: kdelibs Description --- They will no longer exist in KF5, and everyone should be using the Nepomuk widgets instead. Diffs - kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d kio/kfile/kfilemetadatawidget.h 50ddce9 Diff: http://git.reviewboard.kde.org/r/113153/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113157: Remove Nepomuk dependency from kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113157/ --- (Updated Oct. 14, 2013, 4:21 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kdelibs Description --- The only class using it is KFileMetaInfoItem which is just using the ontology parts in order to get a better name for a property. It can rely on strigi instead. Diffs - staging/kde4support/src/CMakeLists.txt 5eb649c staging/kde4support/src/config-kde4support.h.cmake 95a092f staging/kde4support/src/kdebug.areas 2cabd98 staging/kde4support/src/kio/kfilemetainfoitem.cpp 9df2602 staging/kde4support/src/kio/kfilemetainfoitem_p.h 959fbd6 Diff: http://git.reviewboard.kde.org/r/113157/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113154: Remove KFileMetaDataWidget and friends
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113154/ --- (Updated Oct. 14, 2013, 4:21 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kdelibs Description --- Remove KFileMetaDataWidget and friends These have been deprecated in KDE4.[1] This also removes the KFileMetaPropsPlugin in the KPropertiesDialog - The code was commented out so it doesn't really make a difference. Eventually we will need a proper plugin based system so that the Nepomuk2::FileMetadataWidget can be used in the KPropertiesDialog [1] https://git.reviewboard.kde.org/r/113153/ Diffs - KDE5PORTING.html 3171afc kdewidgets/kde.widgets b138d4e staging/kde4support/src/CMakeLists.txt 5eb649c staging/kde4support/src/kio/kcommentwidget.cpp 6223a0c staging/kde4support/src/kio/kcommentwidget_p.h 7a9c710 staging/kde4support/src/kio/kfilemetadataconfigurationwidget.h 52735ad staging/kde4support/src/kio/kfilemetadataconfigurationwidget.cpp 018d183 staging/kde4support/src/kio/kfilemetadataprovider.cpp 59cb238 staging/kde4support/src/kio/kfilemetadataprovider_p.h 0969f53 staging/kde4support/src/kio/kfilemetadatareader.cpp 6a7909c staging/kde4support/src/kio/kfilemetadatareader_p.h af054c2 staging/kde4support/src/kio/kfilemetadatareaderprocess.cpp 0d2b993 staging/kde4support/src/kio/kfilemetadatawidget.h 31dd3c7 staging/kde4support/src/kio/kfilemetadatawidget.cpp 2df2312 staging/kde4support/src/kio/kmetaprops.h b03dd4c staging/kde4support/src/kio/kmetaprops.cpp 46624c5 staging/kde4support/src/kio/knfotranslator.cpp 0494679 staging/kde4support/src/kio/knfotranslator_p.h ddbe4a1 staging/kio/src/widgets/kpropertiesdialog.cpp 63e4435 Diff: http://git.reviewboard.kde.org/r/113154/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Fwd: Failing Tests in Kross
Hey everyone I managed to contact Sebastian through his kdab email account. I've attached the response below. On Fri, Oct 18, 2013 at 3:13 AM, Sebastian Sauer wrote: > On 10/16/2013 03:15 PM, Vishesh Handa wrote: >> >> Hey Sebastian > > > Aloha Vishesh :) > > >> If you get some free time could you please have a look at the tests? > > > Unfortunately I don't have a working kf5 setup, there are no packages for my > distribution and keeping up with kf5 development is near impossible for me > having a fulltime-job and family. > > >> The following tests FAILED: >>1 - kross-guiform-py (Failed) >>2 - kross-guiform-rb (Failed) >>3 - kross-guiqt-py (Failed) >>4 - kross-guiqt-rb (Failed) >>5 - kross-guitk-py (Failed) >>7 - kross-test-py (Failed) > > > hmmm... > >>8 - kross-unittest-es (SEGFAULT) > > > *.es is QtScript. There may have been some incompatible changes in Qt5. > >>9 - kross-unittest-js (SEGFAULT) > > > *.js is KJS and KjsEmbed. Just delete them. They never got into a quality > state. > > >> 10 - kross-unittest-py (Failed) >> 11 - kross-unittest-rb (Failed) > > > Ruby and Python. I saw some weeks ago that Red Hat did some patches to port > the ruby backend to latest ruby. Not sure who took over Python but there > where a few patches last years from others. > > >> The test in the qts plugin also segfaults. > > > That's Qt5 QtScript? hmpf :( > > >> The python and ruby tests fail >> because of the missing kross plugin. > > > Those are the only two important backends. > > >> However the js/es ones seem to segault. > > > Delete it (the test and kjs-backend in kross) and also delete kjsembed from > kdelibs. iirc I was official(?) maintainer of all of that :) Rich Moore, who > pushed it to kdelibs back then, suggested deletion at the kjsembed > mailinglist some years ago, then Matt Bratstone and me took over and I think > I am the only one left. > > >> Do you think you could have a look at them? I'm not comfortable moving it >> to >> tier3 with failing tests. > > > Unfortunately I have no time and while I hack a lot on Qt its 99% Qt4 > related. So, even my Qt5 setup is outdated already (for kf5 which iirc > requires latest/greatest Qt 5.2) and so is my cmake, cmakeextras, all > dependencies, etc. kf5 moves to fast if you only have 1h/week so I decided > to better spend my time on Calligra. > > >> There also seems to be a .ui file in the tests directory, which I'm not >> sure >> how to run. > > > Those are not for run as in auto-tests but are manual tests used in the > kross-gui* scripts (which are not auto-tests as in unittests too but > manual-tests as in start a GUI for clicking and testing manually). > > >> When Kross moves to its own git repo we should probably move the ruby and >> python plugin in it as well. > > > Makes sense. > > >> Also, can I add a MAINTAINERS file with your name in it? > > > I don't think I will have enough time anytime soon. Probably once there are > kf5 packages or once kross can be build standalone without kf5 dependencies > (it used to be that way, means tear1, when I hacked last on it). > > -- > Best regards > > Sebastian Sauer | sebastian.sa...@kdab.com | Software Engineer > KDAB (Deutschland) GmbH&Co KG, a KDAB Group company > Germany: +49-30-521325470, Sweden (HQ): +46-563-540090 > KDAB - Qt Experts - Platform-independent software solutions -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Fwd: Failing Tests in Kross
Aloha Sebastian :D On Fri, Oct 18, 2013 at 3:13 AM, Sebastian Sauer wrote: > > Aloha Vishesh :) > > Unfortunately I don't have a working kf5 setup, there are no packages for my > distribution and keeping up with kf5 development is near impossible for me > having a fulltime-job and family. > That's alright. > >> The following tests FAILED: >>1 - kross-guiform-py (Failed) >>2 - kross-guiform-rb (Failed) >>3 - kross-guiqt-py (Failed) >>4 - kross-guiqt-rb (Failed) >>5 - kross-guitk-py (Failed) >>7 - kross-test-py (Failed) > > > hmmm... > >>8 - kross-unittest-es (SEGFAULT) > > > *.es is QtScript. There may have been some incompatible changes in Qt5. > >>9 - kross-unittest-js (SEGFAULT) > > > *.js is KJS and KjsEmbed. Just delete them. They never got into a quality > state. Alright. I'll remove them. > > >> 10 - kross-unittest-py (Failed) >> 11 - kross-unittest-rb (Failed) > > > Ruby and Python. I saw some weeks ago that Red Hat did some patches to port > the ruby backend to latest ruby. Not sure who took over Python but there > where a few patches last years from others. > > >> However the js/es ones seem to segault. > > > Delete it (the test and kjs-backend in kross) and also delete kjsembed from > kdelibs. iirc I was official(?) maintainer of all of that :) Rich Moore, who > pushed it to kdelibs back then, suggested deletion at the kjsembed > mailinglist some years ago, then Matt Bratstone and me took over and I think > I am the only one left. > KJsEmbed has been currently moved to tier3. Considering that it also doesn't have a maintainer, maybe we should just let it die instead of removing it? Opinions? > > Unfortunately I have no time and while I hack a lot on Qt its 99% Qt4 > related. So, even my Qt5 setup is outdated already (for kf5 which iirc > requires latest/greatest Qt 5.2) and so is my cmake, cmakeextras, all > dependencies, etc. kf5 moves to fast if you only have 1h/week so I decided > to better spend my time on Calligra. > Of course. A large part of that one hour would also be spent in getting everything to compile. >> Also, can I add a MAINTAINERS file with your name in it? > > > I don't think I will have enough time anytime soon. Probably once there are > kf5 packages or once kross can be build standalone without kf5 dependencies > (it used to be that way, means tear1, when I hacked last on it). > We'll need to find another maintainer then :/ > -- > Best regards > > Sebastian Sauer | sebastian.sa...@kdab.com | Software Engineer > KDAB (Deutschland) GmbH&Co KG, a KDAB Group company > Germany: +49-30-521325470, Sweden (HQ): +46-563-540090 > KDAB - Qt Experts - Platform-independent software solutions -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113371: Kross: Remove the KJs backened
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113371/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- Kross: Remove the KJs backened The unit tests currently segfault, and according to the maintainer (Sebastian Sauer) the kjs backend never reached a quality state and should be removed. Diffs - staging/kross/Mainpage.dox c17e1a2 staging/kross/autotests/CMakeLists.txt c5b4a05 staging/kross/autotests/testkross.js e8e7729 staging/kross/autotests/unittest.js 0d7b141 staging/kross/src/CMakeLists.txt 0279846 staging/kross/src/core/action.h e818a51 staging/kross/src/core/interpreter.h d8b1168 staging/kross/src/core/interpreter.cpp 937ca60 staging/kross/src/core/krossconfig.h 0243a53 staging/kross/src/core/manager.h 982817b staging/kross/src/core/manager.cpp 88e312d staging/kross/src/kjs/CMakeLists.txt 32ba00b staging/kross/src/kjs/kjsinterpreter.h be2d61a staging/kross/src/kjs/kjsinterpreter.cpp d89e42b staging/kross/src/kjs/kjsscript.h f760327 staging/kross/src/kjs/kjsscript.cpp 541caf2 Diff: http://git.reviewboard.kde.org/r/113371/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113371: Kross: Remove the KJs backened
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113371/ --- (Updated Oct. 29, 2013, 11:13 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Kross: Remove the KJs backened The unit tests currently segfault, and according to the maintainer (Sebastian Sauer) the kjs backend never reached a quality state and should be removed. Diffs - staging/kross/Mainpage.dox c17e1a2 staging/kross/autotests/CMakeLists.txt c5b4a05 staging/kross/autotests/testkross.js e8e7729 staging/kross/autotests/unittest.js 0d7b141 staging/kross/src/CMakeLists.txt 0279846 staging/kross/src/core/action.h e818a51 staging/kross/src/core/interpreter.h d8b1168 staging/kross/src/core/interpreter.cpp 937ca60 staging/kross/src/core/krossconfig.h 0243a53 staging/kross/src/core/manager.h 982817b staging/kross/src/core/manager.cpp 88e312d staging/kross/src/kjs/CMakeLists.txt 32ba00b staging/kross/src/kjs/kjsinterpreter.h be2d61a staging/kross/src/kjs/kjsinterpreter.cpp d89e42b staging/kross/src/kjs/kjsscript.h f760327 staging/kross/src/kjs/kjsscript.cpp 541caf2 Diff: http://git.reviewboard.kde.org/r/113371/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113496: Move Kross from staging to tier3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113496/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- Move Kross from staging to tier3. Diffs - staging/CMakeLists.txt 808ee92 staging/kross/CMakeLists.txt staging/kross/KrossConfig.cmake.in staging/kross/Mainpage.dox staging/kross/autotests/CMakeLists.txt staging/kross/autotests/main.cpp staging/kross/autotests/profile.py staging/kross/autotests/testguiform.py staging/kross/autotests/testguiform.rb staging/kross/autotests/testguiform.ui staging/kross/autotests/testguiqt.py staging/kross/autotests/testguiqt.rb staging/kross/autotests/testguitk.py staging/kross/autotests/testkross.py staging/kross/autotests/testobject.h staging/kross/autotests/testobject.cpp staging/kross/autotests/unittest.es staging/kross/autotests/unittest.py staging/kross/autotests/unittest.rb staging/kross/docs/CHANGES staging/kross/src/CMakeLists.txt staging/kross/src/console/CMakeLists.txt staging/kross/src/console/main.cpp staging/kross/src/core/CMakeLists.txt staging/kross/src/core/action.h staging/kross/src/core/action.cpp staging/kross/src/core/actioncollection.h staging/kross/src/core/actioncollection.cpp staging/kross/src/core/childreninterface.h staging/kross/src/core/errorinterface.h staging/kross/src/core/interpreter.h staging/kross/src/core/interpreter.cpp staging/kross/src/core/krossconfig.h staging/kross/src/core/krossconfig.cpp staging/kross/src/core/manager.h staging/kross/src/core/manager.cpp staging/kross/src/core/metafunction.h staging/kross/src/core/metatype.h staging/kross/src/core/object.h staging/kross/src/core/object.cpp staging/kross/src/core/script.h staging/kross/src/core/script.cpp staging/kross/src/core/wrapperinterface.h staging/kross/src/modules/CMakeLists.txt staging/kross/src/modules/form.h staging/kross/src/modules/form.cpp staging/kross/src/modules/translation.h staging/kross/src/modules/translation.cpp staging/kross/src/qts/CMakeLists.txt staging/kross/src/qts/interpreter.h staging/kross/src/qts/interpreter.cpp staging/kross/src/qts/main.cpp staging/kross/src/qts/plugin.h staging/kross/src/qts/plugin.cpp staging/kross/src/qts/script.h staging/kross/src/qts/script.cpp staging/kross/src/qts/test.es staging/kross/src/qts/values_p.h staging/kross/src/ui/CMakeLists.txt staging/kross/src/ui/model.h staging/kross/src/ui/model.cpp staging/kross/src/ui/plugin.h staging/kross/src/ui/plugin.cpp staging/kross/src/ui/view.h staging/kross/src/ui/view.cpp tier3/CMakeLists.txt 249a60b Diff: http://git.reviewboard.kde.org/r/113496/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113496: Move Kross from staging to tier3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113496/ --- (Updated Oct. 31, 2013, 10:51 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Move Kross from staging to tier3. Diffs - staging/CMakeLists.txt 808ee92 staging/kross/CMakeLists.txt staging/kross/KrossConfig.cmake.in staging/kross/Mainpage.dox staging/kross/autotests/CMakeLists.txt staging/kross/autotests/main.cpp staging/kross/autotests/profile.py staging/kross/autotests/testguiform.py staging/kross/autotests/testguiform.rb staging/kross/autotests/testguiform.ui staging/kross/autotests/testguiqt.py staging/kross/autotests/testguiqt.rb staging/kross/autotests/testguitk.py staging/kross/autotests/testkross.py staging/kross/autotests/testobject.h staging/kross/autotests/testobject.cpp staging/kross/autotests/unittest.es staging/kross/autotests/unittest.py staging/kross/autotests/unittest.rb staging/kross/docs/CHANGES staging/kross/src/CMakeLists.txt staging/kross/src/console/CMakeLists.txt staging/kross/src/console/main.cpp staging/kross/src/core/CMakeLists.txt staging/kross/src/core/action.h staging/kross/src/core/action.cpp staging/kross/src/core/actioncollection.h staging/kross/src/core/actioncollection.cpp staging/kross/src/core/childreninterface.h staging/kross/src/core/errorinterface.h staging/kross/src/core/interpreter.h staging/kross/src/core/interpreter.cpp staging/kross/src/core/krossconfig.h staging/kross/src/core/krossconfig.cpp staging/kross/src/core/manager.h staging/kross/src/core/manager.cpp staging/kross/src/core/metafunction.h staging/kross/src/core/metatype.h staging/kross/src/core/object.h staging/kross/src/core/object.cpp staging/kross/src/core/script.h staging/kross/src/core/script.cpp staging/kross/src/core/wrapperinterface.h staging/kross/src/modules/CMakeLists.txt staging/kross/src/modules/form.h staging/kross/src/modules/form.cpp staging/kross/src/modules/translation.h staging/kross/src/modules/translation.cpp staging/kross/src/qts/CMakeLists.txt staging/kross/src/qts/interpreter.h staging/kross/src/qts/interpreter.cpp staging/kross/src/qts/main.cpp staging/kross/src/qts/plugin.h staging/kross/src/qts/plugin.cpp staging/kross/src/qts/script.h staging/kross/src/qts/script.cpp staging/kross/src/qts/test.es staging/kross/src/qts/values_p.h staging/kross/src/ui/CMakeLists.txt staging/kross/src/ui/model.h staging/kross/src/ui/model.cpp staging/kross/src/ui/plugin.h staging/kross/src/ui/plugin.cpp staging/kross/src/ui/view.h staging/kross/src/ui/view.cpp tier3/CMakeLists.txt 249a60b Diff: http://git.reviewboard.kde.org/r/113496/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113821: Don't install kpagedialog_p.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113821/#review43572 --- Ship it! I don't spot anything wrong. - Vishesh Handa On Nov. 12, 2013, 6:46 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113821/ > --- > > (Updated Nov. 12, 2013, 6:46 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kdelibs > > > Description > --- > > This makes sure that KWidgetsAddons doesn't expose any private dependencies. > > The only user of that file was KCMultiDialogPrivate. This patch refactors the > code so that it's used separately. > > > Diffs > - > > tier1/kwidgetsaddons/src/CMakeLists.txt 76679a4 > tier4/kcmutils/src/kcmultidialog.h 3518736 > tier4/kcmutils/src/kcmultidialog.cpp 9d2ccbb > tier4/kcmutils/src/kcmultidialog_p.h ad5dd98 > > Diff: http://git.reviewboard.kde.org/r/113821/diff/ > > > Testing > --- > > Everything builds, couldn't test since I didn't see any test. > > Tests still pass though. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115606: Ommit ontologies/ dir from installing
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115606/#review49423 --- As far as I'm concerned it can be nuked. But let's wait for Ivan. - Vishesh Handa On Feb. 9, 2014, 7:58 p.m., Hrvoje Senjan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115606/ > --- > > (Updated Feb. 9, 2014, 7:58 p.m.) > > > Review request for KDE Frameworks and Ivan Čukić. > > > Repository: kactivities > > > Description > --- > > So kf5 kactivities can be co-installed with kactivities4. First approach is > just to comment out the dir. I don't know what are the plans, so potentially > i can git rm the dir, considering the emerge of baloo with 4.13 > > > Diffs > - > > src/CMakeLists.txt 8a00cf8 > > Diff: https://git.reviewboard.kde.org/r/115606/diff/ > > > Testing > --- > > Builds. > > > Thanks, > > Hrvoje Senjan > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: let's get ready for Google Summer of Code 2014
On Monday, February 10, 2014 01:54:36 PM Mark Gaiser wrote: > Done: > http://community.kde.org/GSoC/2014/Ideas#Revive_KioFuse.2C_fuse_support_for > _KIO > > Lets hope a student comes by and picks that project :) > All we need then is someone to mentor that. > Not you? -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Please add new versions on bugs.kde.org products on KF5 releases
On Sat, Apr 9, 2016 at 12:09 PM, David Faure wrote: > One following issue remains: > there is no frameworks-krunner bugzilla product. There is a krunner product, > but it's not the same > thing, it's not KF5-versionned. Should we add one? Who would be the > maintainer? > (the yaml file points to Vishesh). Whatever is easier. I am the maintainer, but I haven't been doing anything, so someone else should take over. I'll write an email announcing that soon. -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Find out whether a file is a remote file or not
On Sun, May 1, 2016 at 6:28 PM, Dominik Haumann wrote: > > CC: Vishesh, since maybe baloo also had to solve these issues We mostly just use Solid to query the list of mount points and ignore non local stuff. baloo/src/file/storagedevices.h -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 128892: Open baloo lmdb database read-only beside in baloo_file/baloo_file_extractor + balooctl (for some commands) + unit tests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128892/#review99115 --- Ship it! This is awesome. Ship it. - Vishesh Handa On Sept. 11, 2016, 7:19 p.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128892/ > --- > > (Updated Sept. 11, 2016, 7:19 p.m.) > > > Review request for KDE Frameworks, Plasma, Boudhayan Gupta, and Vishesh Handa. > > > Repository: baloo > > > Description > --- > > At the moment, any application that uses baloo can corrupt the db. > Now, only the things that need to write to it open it with read-write. > This only works as long as the library exposes only read-only things like > Query/... > > > Diffs > - > > src/engine/database.h 6ccb2a5 > src/engine/database.cpp 6a433c7 > src/file/extractor/app.cpp 0ca7276 > src/lib/file.cpp cbbc912 > src/lib/searchstore.cpp 060a4fd > src/lib/taglistjob.cpp 76ac8ff > src/qml/experimental/monitor.cpp 11c06ae > src/tools/balooctl/main.cpp 2a6b175 > src/tools/balooctl/statuscommand.cpp 1a56c64 > src/tools/balooshow/main.cpp f45f2e0 > > Diff: https://git.reviewboard.kde.org/r/128892/diff/ > > > Testing > --- > > Unit tests still work, balooctl seems to do something. > > > Thanks, > > Christoph Cullmann > >
Scrap Baloo Thread Feedback
Hey guys I was told there is a thread about scrapping Baloo. All Baloo discussion used to happen on kde-devel and that's where the review requests go. It's the only reason I am still subscribed to kde-devel. I must say, the thread is overall quite disappointing. There seems to be no scientific or rationale cost based analysis of this. How about a list of requirements and priorities are drawn up and then possible solutions are evaluated according to it? Right now, random requirements such as NFS and 32bit systems are coming up. Are these really that important? I specifically designed Baloo to not care about both network mounts and 32-bit systems. Yes, Baloo has bugs and it won't handle more than 32bit-inodes. These things, as all others, can be fixed. It's really a question of what is important. Lets not target the outliers. Many of these decisions were deliberately taken. How about requirements such as resource consumption, ease of integration, search speed are taken into consideration? Come on guys. We're engineers over here. (If the discussion continues on kde-frameworks-devel, I probably won't see it) -- Vishesh Handa
Re: Scrap Baloo Thread Feedback
On Fri, Oct 7, 2016 at 5:57 PM, Kevin Funk wrote: > On Friday, 7 October 2016 17:24:26 CEST Vishesh Handa wrote: >> Hey guys >> >> I was told there is a thread about scrapping Baloo. All Baloo >> discussion used to happen on kde-devel and that's where the review >> requests go. It's the only reason I am still subscribed to kde-devel. > > Heya, > > Baloo is a framework nowadays, therefore it totally makes sense to have the > discussion on kde-framework-devel. > > There's been tons of discussion around Baloo on kde-framework-devel already. > kde-frameworks-devel is also where the CI messages for the baloo repo go to. > > It likely makes sense for you to subscribe, no? > I don't understand why all framework discussions must happen on the same list. It just adds to a crazy amount of noise, which one then needs to parse through. > Cheers, > Kevin > >> (snip) > > > -- > Kevin Funk | kf...@kde.org | http://kfunk.org
Re: Scrap Baloo Thread Feedback
On Fri, Oct 7, 2016 at 6:14 PM, Christoph Cullmann wrote: >>> >> >> I don't understand why all framework discussions must happen on the >> same list. It just adds to a crazy amount of noise, which one then >> needs to parse through. > > If you would have baloo-devel I could understand that point, > but not with some other generic mailing list like kde-devel which > has the same amount of noise and is not even dedicated to 'frameworks' > or 'baloo'. If you guys plans to use frameworks devel, then please change the review requests. It was just too much noise for me, and I found the noise/signal ratio way lower in kde-devel. Baloo-devel was specifically not chosen as it would just an another silo in kde. Nepomuk used to suffer from that. -- Vishesh Handa
Re: Scrap Baloo Thread Feedback
On Fri, Oct 7, 2016 at 6:20 PM, Aleix Pol wrote: >>> >> >> I don't understand why all framework discussions must happen on the >> same list. It just adds to a crazy amount of noise, which one then >> needs to parse through. > > Arguing that it should be elsewhere because you'd like to ignore the > rest of the traffic in kde-frameworks doesn't sound very constructive, > especially considering how they're the "noise" that actually improves > the frameworks. > > Maybe you can better configure your e-mail client differently so we > can focus on the issue at matter? This is not about how it should be. I'm informing them why it was chosen to be somewhere else. This decision can be changed. Frameworks collectively may or may not improve by having everything in one place. Lets not treat it as a axiom. An analogy could be that we get commit emails, but we get to choose which projects we are interested in. We don't make everyone subscribe to kde-commits, and then put their own complex filters on top. -- Vishesh Handa
Re: Scrap Baloo Thread Feedback
Hey On Fri, Oct 7, 2016 at 6:34 PM, Christoph Cullmann wrote: > Hi, > >> On Fri, Oct 7, 2016 at 5:58 PM, Christoph Cullmann >> wrote: >>> > > 1) No handling of DB errors beside asserting > 2) No handling of errors in the extractors (e.g. see the fixes I did, all > extractors will need more of that) > 3) No handling of NFS/large inodes/inconsistencies => crash > > In the end, in my opinion, you can rewrite close to all parts dealing with > the DB or > any other thing internally. If ever any thing gots inconsistent, ATM you are > doomed, forever, > if not by luck my new startup code deletes the index, then you live again > until it is reindexed. > >> > I am not sure, I am all for removing complete indexing and use a other indexer > like tracker to exactly avoid the excurse into DB world and how to handle it > in a safe way with close to zero person manpower. > It's avoiding the problem and hoping for the best, without any experiments. > > => That is good that we agree, but I find it very astonishing that we use > baloo in its > current state more or less mandatory on all that systems were it by design > will fail. > > (and it fails if you read the bugs) > There is a certain amount of failure, but it's not "by-design". But maybe I'm not seeing things clearly. >> >>>> >>>> How about requirements such as resource consumption, ease of >>>> integration, search speed are taken into consideration? Come on guys. >>>> We're engineers over here. >>> What is the argument here? If you take a look at bugs.kde.org, you see that >>> people are complaining about all >>> of that with baloo. I see no evidence nowhere that e.g. baloo is "superior" >>> to >>> what GNOME uses >>> or any other solution (perhaps beside nepomuk, ok...). What tests have been to obtain the evidence? > >> >> Yup, you have. It's awesome. I no longer have the motivation to work on >> Baloo. > Thanks, but that makes me very sad, btw. > Baloo came up to replace nepomuk, which was dead because it had too many bugs > and all maintainers left. > Now we have baloo, which has many bugs, some even by design, and the > maintainer left, too. > Actually, Nepomuk was not dead. I was maintaining it. I killed it because it had too many structural problems. This is how the open source world works. People work on projects and when it no longer scratches their itch (I no longer use Baloo), they loose interest. This is "supposed" to be a hobby. > >> (This is why they run on a separate process) > That doesn't help, it just OOMs your system => dead, it needs resource > restrictions, > which is tricky to get right. > You're right. It needs a better thought out solution. A separate process is the bare minimum. Btw, have you looked if Tracker actually does any of this? >> My hostility was because the proposal ignores key points such as - >> >> * Indexing Speed >> * Search speed >> * Database size > => If you look at the bugs, people complain we are inferior and I see not > that the proposal ignores it, I just see not how to compare, given there are > no > hard facts that we are faster than e.g. tracker in any way. > Data can be gathered about it. Not all data is publicly available. >> * Ease of use with our existing components > My proposal did not change the interface at all, it has zero impact on "ease > of use". > >> * Ease of fixing problems in the code > My estimate would be: rewrite close to everything. Even the basic 64-bit int > id won't work > with 64-bit inodes, each DB call must be touched to check for errors, at each > place > one will need to check for potential inconsistencies and exit gracefully... > I don't follow why everything needs to be re-written? Am I missing something or do we just need to check for more errors and use a higher integer id? This certainly doesn't seem super trivial, but it sounds like less work than implementing a shim on top of Tracker. I could be wrong. >> >> Baloo has certain speed requirements if it is to be used with krunner, >> and we want instant feedback. This was an integral requirement. > I doubt e.g. tracker has different requirements, as it is used in similar > places by GNOME. > > But all that left besides, have you an proposal how to fixup the current > situation? > Are you willing to invest some work to fix the current issues or an idea what > would be a good way to tackle them? > I probably will not work more in Baloo. I'll have to investigate the problems a bit more. From the cursory look of this thread, it doesn't seem that the problems are that dire. But I may not be reading into it correctly. -- Vishesh Handa
Re: Scrap Baloo Thread Feedback
On Fri, Oct 7, 2016 at 8:08 PM, Christoph Cullmann wrote:
>
> I did experiments and search works with tracker, but yes, a problem is
> tagging,+
> which ATM doesn't work. Nor do I say that is a ready solution now, just a
> possibility
> to avoid having to maintain low level code with at most 1 person (how it
> looks ATM).
>
> And I don't propose to go that road now, but ATM I see nobody doing any other
> experiments.
>
> Besides, tracker is constantly maintained and used since >> 5 years:
>
> https://github.com/GNOME/tracker/graphs/contributors
ok. Baloo clearly isn't being maintained.
>
>>
>>>
>>> => That is good that we agree, but I find it very astonishing that we use
>>> baloo
>>> in its
>>> current state more or less mandatory on all that systems were it by design
>>> will
>>> fail.
>>>
>>> (and it fails if you read the bugs)
>>>
>>
>> There is a certain amount of failure, but it's not "by-design". But
>> maybe I'm not seeing things clearly.
> You yourself stated that neither 32-bit issues nor NFS nor > 32-bit inodes
> have any
> error handling. And that seems to have been known even during design and still
> we have this now as a framework per default used by any Plasma installation on
> systems exactly featuring that without error checking.
>
>>
>>
>> How about requirements such as resource consumption, ease of
>> integration, search speed are taken into consideration? Come on guys.
>> We're engineers over here.
>>
> What is the argument here? If you take a look at bugs.kde.org, you see
> that
> people are complaining about all
> of that with baloo. I see no evidence nowhere that e.g. baloo is
> "superior" to
> what GNOME uses
> or any other solution (perhaps beside nepomuk, ok...).
>>
>> What tests have been to obtain the evidence?
> What tests have been done to obtain the inverse evidence? I only hear here
> the complaint
> about not taking requirements like resource consumption or speed into
> account, but
> there is ATM zero evidence that e.g. tracker is slower.
>
I did do a lot of tests during the design of Baloo. I don't have hard numbers.
Even if I didn't, that doesn't mean a decision should be made without
gathering proper data.
> And the typical error check is:
>
> void MTimeDB::put(quint32 mtime, quint64 docId)
> {
> Q_ASSERT(mtime > 0);
> Q_ASSERT(docId > 0);
>
> MDB_val key;
> key.mv_size = sizeof(quint32);
> key.mv_data = static_cast(&mtime);
>
> MDB_val val;
> val.mv_size = sizeof(quint64);
> val.mv_data = static_cast(&docId);
>
> int rc = mdb_put(m_txn, m_dbi, &key, &val, 0);
> Q_ASSERT_X(rc == 0, "MTimeDB::put", mdb_strerror(rc));
> }
>
> without any way to pass an error to the outside, nor any error handling code
> at the outside,
> as no error can ever occur that is non-fatal.
>
ok. The API isn't exported. It can be changed.
But we both seem to have different opinions of how much work this would be.
>
> Besides I don't see any documentation of the DB format, but I could miss that.
> (at least not in the git nor https://community.kde.org/Baloo)
>
There isn't any.
>
> What would be highly appreciated would be a bit of documentation what the
> different pieces do and stuff like that, even if you have no time to code.
>
If you can send me specific questions about different parts I can
answer them. For "general documentation", I don't know where to start.
I usually much prefer just going through the code.
> Greetings
> Christoph
>
Re: Scrap Baloo Thread Feedback
On Sun, Oct 16, 2016 at 2:16 PM, Christoph Cullmann wrote: > Hi, > > (evil top posting) > > given the silence, I assume any interest in baloo has stopped once more, or? > Or are there any plans how to fixup the current situation? I'm not going to be very involved with this. I've already expressed my opinion. You all are free to make a decision. -- Vishesh Handa
Re: Review Request 109538: port KFileMetaDataReader to QProcess
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109538/#review29377 --- But why? KFileMetadataReader and the other KFileMetadataStuff should just be marked as deprecated. Why are we porting them? We already have better alternatives in the nepomuk-widgets repository. - Vishesh Handa On March 17, 2013, 1:26 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109538/ > --- > > (Updated March 17, 2013, 1:26 p.m.) > > > Review request for KDE Frameworks, kdelibs, David Faure, and Vishesh Handa. > > > Description > --- > > KFileMetaDataReader currently uses KProcess, this ports it to use QProcess > instead. > > > Diffs > - > > kio/kfile/kfilemetadatareader.cpp 88cadaa > > Diff: http://git.reviewboard.kde.org/r/109538/diff/ > > > Testing > --- > > it builds. > > > Thanks, > > Martin Tobias Holmedahl Sandsmark > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 109538: port KFileMetaDataReader to QProcess
> On March 17, 2013, 2:05 p.m., Vishesh Handa wrote: > > But why? KFileMetadataReader and the other KFileMetadataStuff should just > > be marked as deprecated. Why are we porting them? We already have better > > alternatives in the nepomuk-widgets repository. > > Martin Tobias Holmedahl Sandsmark wrote: > Because it was a simple user of KProcess. > > But if we can just deprecate the whole class (and move it into > kde4support, I guess?) that's better. :-) As far as I'm concerned it can be deprecated. We can definitely deprecate KFileMetadataWidget which is the only user of this KFileMetadataReader. Dolphin now uses Nepomuk2::FileMetadataWidget. The only slight problem might be that Dolphin still likes being "Nepomuk Optional" at compile time. So they still use it. Maybe we should talk to Frank about this? The only other class is KFileMetaInfo, which uses Strigi directly. I still have to write a replacement for that in Nepomuk. @David: I think we talked about this in Berlin. Should we deprecate KFileMetadataWidget? - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109538/#review29377 --- On March 17, 2013, 1:26 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109538/ > --- > > (Updated March 17, 2013, 1:26 p.m.) > > > Review request for KDE Frameworks, kdelibs, David Faure, and Vishesh Handa. > > > Description > --- > > KFileMetaDataReader currently uses KProcess, this ports it to use QProcess > instead. > > > Diffs > - > > kio/kfile/kfilemetadatareader.cpp 88cadaa > > Diff: http://git.reviewboard.kde.org/r/109538/diff/ > > > Testing > --- > > it builds. > > > Thanks, > > Martin Tobias Holmedahl Sandsmark > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Fancy header policy
Hey guys I've been looking at the frameworks branch and I cannot help but notice that there is an 'include' folder which contains all the fancy headers. Is there any policy on what needs to be done about it? Some of the fancy headers, such as the Nepomuk ones, should just be discarded. Should the others be moved to their respective frameworks in a special include folder? So 'include/Solid/*' would move to 'tier1/solid/includes/' ? Also, under what folder name should there includes be installed? Currently the fancy includes are installed in a KDE folder (/include/KDE/fancyHeaders). Do we want to continue with that? Or just install them in the specific folder. Eg - Solid headers would now be installed in include/Solid/ instead of include/KDE/Solid/ Also, dibs! -- Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111897: Move KFileMetaData (and friends) to kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111897/#review37193 --- I was just working on the same thing. I'm not sure if we want to move this to kde4support. Can we just throw it away? Or would that be terribly wrong? We have a replacement in nepomuk-widgets. Strigi doesn't need to be ported to Qt5 since it is does not use Qt. Soprano will have to be, but I don't think this code uses Soprano. - Vishesh Handa On Aug. 5, 2013, 6:06 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111897/ > --- > > (Updated Aug. 5, 2013, 6:06 p.m.) > > > Review request for KDE Frameworks and Vishesh Handa. > > > Description > --- > > As far as I've understood, we should have an alternative by Nepomuk for file > previewing for KF5, this patch moves the KFileMetaInfo and the files that > depend on it to KDE4Support. > > It's worth noting that there are 2 "plugins" (they're actually not plugins) > of the KPropertiesDialog that have been disabled because they had to be moved > with KFileMetaInfo. That is the kmetaprops.cpp and kpreviewprops.cpp > > > Diffs > - > > kio/CMakeLists.txt 035cf70 > kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d > kio/kfile/kfilemetadataconfigurationwidget.cpp > kio/kfile/kfilemetadataprovider.cpp > kio/kfile/kfilemetadataprovider_p.h 8009bf4 > kio/kfile/kfilemetadatareader.cpp > kio/kfile/kfilemetadatareader_p.h > kio/kfile/kfilemetadatareaderprocess.cpp > kio/kfile/kfilemetadatawidget.h 2dc4677 > kio/kfile/kfilemetadatawidget.cpp > kio/kfile/kmetaprops.h a08c380 > kio/kfile/kmetaprops.cpp > kio/kfile/knfotranslator.cpp > kio/kfile/knfotranslator_p.h > kio/kfile/kpreviewprops.h 8a974da > kio/kfile/kpreviewprops.cpp > kio/kfile/kpropertiesdialog.cpp 687e4bf > staging/kde4support/src/CMakeLists.txt 1d6369f > staging/kde4support/src/config-kde4support.h.cmake 03d3bf4 > > Diff: http://git.reviewboard.kde.org/r/111897/diff/ > > > Testing > --- > > builds... actually i'm not sure if there's Qt5 soprano/strigi. what's hte > status? > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111897: Move KFileMetaData (and friends) to kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111897/#review37195 --- You seem to have forgotten about kcommentwidget. It can be discarded/moved as well. - Vishesh Handa On Aug. 5, 2013, 6:06 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111897/ > --- > > (Updated Aug. 5, 2013, 6:06 p.m.) > > > Review request for KDE Frameworks and Vishesh Handa. > > > Description > --- > > As far as I've understood, we should have an alternative by Nepomuk for file > previewing for KF5, this patch moves the KFileMetaInfo and the files that > depend on it to KDE4Support. > > It's worth noting that there are 2 "plugins" (they're actually not plugins) > of the KPropertiesDialog that have been disabled because they had to be moved > with KFileMetaInfo. That is the kmetaprops.cpp and kpreviewprops.cpp > > > Diffs > - > > kio/CMakeLists.txt 035cf70 > kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d > kio/kfile/kfilemetadataconfigurationwidget.cpp > kio/kfile/kfilemetadataprovider.cpp > kio/kfile/kfilemetadataprovider_p.h 8009bf4 > kio/kfile/kfilemetadatareader.cpp > kio/kfile/kfilemetadatareader_p.h > kio/kfile/kfilemetadatareaderprocess.cpp > kio/kfile/kfilemetadatawidget.h 2dc4677 > kio/kfile/kfilemetadatawidget.cpp > kio/kfile/kmetaprops.h a08c380 > kio/kfile/kmetaprops.cpp > kio/kfile/knfotranslator.cpp > kio/kfile/knfotranslator_p.h > kio/kfile/kpreviewprops.h 8a974da > kio/kfile/kpreviewprops.cpp > kio/kfile/kpropertiesdialog.cpp 687e4bf > staging/kde4support/src/CMakeLists.txt 1d6369f > staging/kde4support/src/config-kde4support.h.cmake 03d3bf4 > > Diff: http://git.reviewboard.kde.org/r/111897/diff/ > > > Testing > --- > > builds... actually i'm not sure if there's Qt5 soprano/strigi. what's hte > status? > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111897: Move KFileMetaData (and friends) to kde4support
> On Aug. 6, 2013, 9 a.m., Vishesh Handa wrote: > > I was just working on the same thing. > > > > I'm not sure if we want to move this to kde4support. Can we just throw it > > away? Or would that be terribly wrong? We have a replacement in > > nepomuk-widgets. > > > > Strigi doesn't need to be ported to Qt5 since it is does not use Qt. > > Soprano will have to be, but I don't think this code uses Soprano. > > Aleix Pol Gonzalez wrote: > You were working on it? -.- it didn't have your name on it... > > I think that the classes called plugin should be removed, there's not > much else to remove otherwise. I'd just started today morning, then I decided to try and compile everything. It has been 5 hours since then. I'm still compiling. There is just one user visible class - KFileMetadataWidget. The rest of the classes are helper code. A large part of the helper code uses Nepomuk1. If we move this to kde4support, then those Nepomuk1 dependencies have to be removed. Removing them would make this into a wrapper over Strigi. The question is - do we want that? Or do we just want to discard this class completely? Based on [1] there seem to be 3 clients. Dolphin which uses it when Nepomuk compilation is disabled. Conquirere, which is a Nepomuk based app and should just use the one in nepomuk-widgets, and Konversation - I'm not sure what to do about them. If we throw away this class then we will just be breaking Konversation. I'm obviously in favor of discarding the class. Opinions? This also raises the larger question if we want classes in kde4support to depend on unmaintained code? (Strigi) [1] http://lxr.kde.org/ident?i=KFileMetaDataWidget - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111897/#review37193 --- On Aug. 5, 2013, 6:06 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111897/ > ------- > > (Updated Aug. 5, 2013, 6:06 p.m.) > > > Review request for KDE Frameworks and Vishesh Handa. > > > Description > --- > > As far as I've understood, we should have an alternative by Nepomuk for file > previewing for KF5, this patch moves the KFileMetaInfo and the files that > depend on it to KDE4Support. > > It's worth noting that there are 2 "plugins" (they're actually not plugins) > of the KPropertiesDialog that have been disabled because they had to be moved > with KFileMetaInfo. That is the kmetaprops.cpp and kpreviewprops.cpp > > > Diffs > - > > kio/CMakeLists.txt 035cf70 > kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d > kio/kfile/kfilemetadataconfigurationwidget.cpp > kio/kfile/kfilemetadataprovider.cpp > kio/kfile/kfilemetadataprovider_p.h 8009bf4 > kio/kfile/kfilemetadatareader.cpp > kio/kfile/kfilemetadatareader_p.h > kio/kfile/kfilemetadatareaderprocess.cpp > kio/kfile/kfilemetadatawidget.h 2dc4677 > kio/kfile/kfilemetadatawidget.cpp > kio/kfile/kmetaprops.h a08c380 > kio/kfile/kmetaprops.cpp > kio/kfile/knfotranslator.cpp > kio/kfile/knfotranslator_p.h > kio/kfile/kpreviewprops.h 8a974da > kio/kfile/kpreviewprops.cpp > kio/kfile/kpropertiesdialog.cpp 687e4bf > staging/kde4support/src/CMakeLists.txt 1d6369f > staging/kde4support/src/config-kde4support.h.cmake 03d3bf4 > > Diff: http://git.reviewboard.kde.org/r/111897/diff/ > > > Testing > --- > > builds... actually i'm not sure if there's Qt5 soprano/strigi. what's hte > status? > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 111911: Port kioslave/ftp/ftp.cpp away from kde_file.h
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111911/
---
Review request for KDE Frameworks.
Description
---
A couple of issues -
1. KDE::open/stat/etc take a QString and convert it to a char* via
QFile::encodeName(str).constData(). Qt obviously does not have methods to do
so. Instead of me doing it manually for each call to QT_OPEN/QT_STAT/etc, would
be it okay for me to declare local functions called KDE::stat/open?
Something along the lines of -
namespace KDE {
int open(const QString& filePath, ...) {
return QT_OPEN(QFile::encodeName(filePath).constData()), ...);
}
}
2. The kioslave uses KDE::utime to set the utime of file. I've used ::utime,
but that obviously won't work on non-unix platforms. What is the correct
solution?
One option is to add utime in qplatformdefs.h, but that is non trivial since Qt
seems to support about 104 different qplatformdefs and therefore all of them
will have to be updated.
Diffs
-
kioslave/ftp/ftp.cpp a0da54b
Diff: http://git.reviewboard.kde.org/r/111911/diff/
Testing
---
Doesn't even compile right now. With (1) it will start to compile.
Thanks,
Vishesh Handa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp 189a98e Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111897: Move KFileMetaData (and friends) to kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111897/#review37335 --- You're moving kfilemetainfo but not predicateproperties? Isn't predicateproperties used by kfilemetainfo? Same problem with KFileMetaInfoItem and KFileWritePlugin. - Vishesh Handa On Aug. 7, 2013, 4:23 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111897/ > --- > > (Updated Aug. 7, 2013, 4:23 p.m.) > > > Review request for KDE Frameworks and Vishesh Handa. > > > Description > --- > > As far as I've understood, we should have an alternative by Nepomuk for file > previewing for KF5, this patch moves the KFileMetaInfo and the files that > depend on it to KDE4Support. > > It's worth noting that there are 2 "plugins" (they're actually not plugins) > of the KPropertiesDialog that have been disabled because they had to be moved > with KFileMetaInfo. That is the kmetaprops.cpp and kpreviewprops.cpp > > > Diffs > - > > staging/kde4support/autotests/kiotesthelper.h PRE-CREATION > staging/kde4support/autotests/CMakeLists.txt 22da9ae > kioslave/metainfo/metainfo.protocol > kioslave/metainfo/metainfo.cpp > kioslave/metainfo/metainfo.h > kioslave/metainfo/CMakeLists.txt a448576 > kioslave/CMakeLists.txt c16b33e > kio/tests/kmfitest.cpp > kio/tests/kfilemetainfotest.cpp 74d518b > kio/tests/kfilemetainfotest.h > kio/tests/CMakeLists.txt 6e8c6e7 > kio/kio/kfilemetainfoitem_p.h 607ac3e > kio/kio/kfilemetainfoitem.cpp fbaefe0 > kio/kio/kfilemetainfoitem.h b414274 > kio/kio/kfilemetainfo.cpp a87a8a0 > kio/kio/kfilemetainfo.h ddd26d3 > kio/kfile/kpropertiesdialog.cpp 687e4bf > kio/kfile/kpreviewprops.cpp > kio/kfile/kpreviewprops.h 8a974da > kio/kfile/knfotranslator_p.h > kio/kfile/knfotranslator.cpp 64bcecc > kio/kfile/kmetaprops.cpp > kio/kfile/kmetaprops.h a08c380 > kio/kfile/kfilemetadatawidget.cpp 6e0f106 > kio/kfile/kfilemetadatawidget.h 2dc4677 > kio/kfile/kfilemetadatareaderprocess.cpp > kio/kfile/kfilemetadatareader_p.h > kio/kfile/kfilemetadatareader.cpp > kio/kfile/kfilemetadataprovider_p.h 8009bf4 > kio/kfile/kfilemetadataprovider.cpp > kio/kfile/kfilemetadataconfigurationwidget.cpp 5fbf8dc > kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d > kio/kfile/kcommentwidget_p.h > kio/kfile/kcommentwidget.cpp f125bc7 > kio/CMakeLists.txt 0e6c50d > staging/kde4support/src/CMakeLists.txt 4ea3497 > staging/kde4support/src/config-kde4support.h.cmake 03d3bf4 > staging/kde4support/src/kioslave/CMakeLists.txt PRE-CREATION > staging/kde4support/src/kioslave/metainfo/CMakeLists.txt PRE-CREATION > staging/kde4support/tests/CMakeLists.txt 41e35ce > > Diff: http://git.reviewboard.kde.org/r/111897/diff/ > > > Testing > --- > > builds... actually i'm not sure if there's Qt5 soprano/strigi. what's hte > status? > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111897: Move KFileMetaData (and friends) to kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111897/#review37348 --- Ship it! seems good to me, but it would be nice if someone else could also look at it. kio/kfile/kpropertiesdialog.cpp <http://git.reviewboard.kde.org/r/111897/#comment27625> Yes, they will need to be made into plugins. One that nepomuk can provide. @David: I think we talked about this during Akademy? - Vishesh Handa On Aug. 8, 2013, 11:59 a.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111897/ > --- > > (Updated Aug. 8, 2013, 11:59 a.m.) > > > Review request for KDE Frameworks and Vishesh Handa. > > > Description > --- > > As far as I've understood, we should have an alternative by Nepomuk for file > previewing for KF5, this patch moves the KFileMetaInfo and the files that > depend on it to KDE4Support. > > It's worth noting that there are 2 "plugins" (they're actually not plugins) > of the KPropertiesDialog that have been disabled because they had to be moved > with KFileMetaInfo. That is the kmetaprops.cpp and kpreviewprops.cpp > > > Diffs > - > > kio/CMakeLists.txt 89c09ec > kio/kfile/kcommentwidget.cpp f125bc7 > kio/kfile/kcommentwidget_p.h > kio/kfile/kfilemetadataconfigurationwidget.h 6be2a0d > kio/kfile/kfilemetadataconfigurationwidget.cpp 5fbf8dc > kio/kfile/kfilemetadataprovider.cpp > kio/kfile/kfilemetadataprovider_p.h 8009bf4 > kio/kfile/kfilemetadatareader.cpp > kio/kfile/kfilemetadatareader_p.h > kio/kfile/kfilemetadatareaderprocess.cpp > kio/kfile/kfilemetadatawidget.h 2dc4677 > kio/kfile/kfilemetadatawidget.cpp 6e0f106 > kio/kfile/kmetaprops.h a08c380 > kio/kfile/kmetaprops.cpp > kio/kfile/knfotranslator.cpp 64bcecc > kio/kfile/knfotranslator_p.h > kio/kfile/kpreviewprops.h 8a974da > kio/kfile/kpreviewprops.cpp > kio/kfile/kpropertiesdialog.cpp 687e4bf > kio/kio/dummyanalyzers/CMakeLists.txt > kio/kio/dummyanalyzers/dummyanalyzers.cpp > kio/kio/kfilemetainfo.h ddd26d3 > kio/kio/kfilemetainfo.cpp a87a8a0 > kio/kio/kfilemetainfoitem.h b414274 > kio/kio/kfilemetainfoitem.cpp fbaefe0 > kio/kio/kfilemetainfoitem_p.h 607ac3e > kio/kio/kfilewrite.desktop > kio/kio/kfilewriteplugin.h > kio/kio/kfilewriteplugin.cpp > kio/kio/kfilewriteplugin_p.h > kio/kio/predicateproperties.h > kio/kio/predicateproperties.cpp > kio/tests/CMakeLists.txt e6deb62 > kio/tests/kfilemetainfotest.h > kio/tests/kfilemetainfotest.cpp 74d518b > kio/tests/kmfitest.cpp > kioslave/CMakeLists.txt c16b33e > kioslave/metainfo/CMakeLists.txt a448576 > kioslave/metainfo/metainfo.h > kioslave/metainfo/metainfo.cpp > kioslave/metainfo/metainfo.protocol > staging/kde4support/autotests/CMakeLists.txt 22da9ae > staging/kde4support/autotests/kiotesthelper.h PRE-CREATION > staging/kde4support/src/CMakeLists.txt 4ea3497 > staging/kde4support/src/config-kde4support.h.cmake 03d3bf4 > staging/kde4support/src/kioslave/CMakeLists.txt PRE-CREATION > staging/kde4support/src/kioslave/metainfo/CMakeLists.txt PRE-CREATION > staging/kde4support/tests/CMakeLists.txt 41e35ce > > Diff: http://git.reviewboard.kde.org/r/111897/diff/ > > > Testing > --- > > builds... actually i'm not sure if there's Qt5 soprano/strigi. what's hte > status? > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111981: Mark strigi as optional dependancy, move it's check to kde4support and remove FindStrigi.cmake file
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111981/#review37497 --- I'm kinda confused. Strigi is still very much required by kde4support/src/kio/kfilemetainfo.cpp. The only reason this compiles is because Strigi is optional, but without it KFileMetaInfo will completely break. Certain application such as localize still depend on it. We cannot break it without offering an alternative. - Vishesh Handa On Aug. 9, 2013, 9:32 p.m., Hrvoje Senjan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111981/ > --- > > (Updated Aug. 9, 2013, 9:32 p.m.) > > > Review request for KDE Frameworks. > > > Description > --- > > Follow up to r111897. As said in summary - strigi is not *required* anymore. > > > Diffs > - > > CMakeLists.txt 3e8e639 > cmake/modules-tests/RunAllModuleTests.cmake 1ccce7d > cmake/modules-tests/Strigi/CMakeLists.txt bbe1e23 > cmake/modules/CMakeLists.txt 06f5c67 > cmake/modules/FindStrigi.cmake bb87b0d > staging/kde4support/CMakeLists.txt c0e81ad > staging/kde4support/src/CMakeLists.txt bebce74 > > Diff: http://git.reviewboard.kde.org/r/111981/diff/ > > > Testing > --- > > Compiles fine without strigi. > > > Thanks, > > Hrvoje Senjan > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111981: Mark strigi as optional dependancy, move it's check to kde4support and remove FindStrigi.cmake file
> On Aug. 11, 2013, 8:06 a.m., Vishesh Handa wrote: > > I'm kinda confused. > > > > Strigi is still very much required by > > kde4support/src/kio/kfilemetainfo.cpp. The only reason this compiles is > > because Strigi is optional, but without it KFileMetaInfo will completely > > break. Certain application such as localize still depend on it. We cannot > > break it without offering an alternative. Please ignore my comment. I didn't see the new diff. - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111981/#review37497 --- On Aug. 9, 2013, 9:32 p.m., Hrvoje Senjan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111981/ > --- > > (Updated Aug. 9, 2013, 9:32 p.m.) > > > Review request for KDE Frameworks. > > > Description > --- > > Follow up to r111897. As said in summary - strigi is not *required* anymore. > > > Diffs > - > > CMakeLists.txt 3e8e639 > cmake/modules-tests/RunAllModuleTests.cmake 1ccce7d > cmake/modules-tests/Strigi/CMakeLists.txt bbe1e23 > cmake/modules/CMakeLists.txt 06f5c67 > cmake/modules/FindStrigi.cmake bb87b0d > staging/kde4support/CMakeLists.txt c0e81ad > staging/kde4support/src/CMakeLists.txt bebce74 > > Diff: http://git.reviewboard.kde.org/r/111981/diff/ > > > Testing > --- > > Compiles fine without strigi. > > > Thanks, > > Hrvoje Senjan > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 9:35 a.m.) Review request for KDE Frameworks. Changes --- Used QFileInfo instead of QT_STAT/LSTAT Description --- Port khtml_part away from kde_file.h Diffs (updated) - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
> On Aug. 12, 2013, 9:40 a.m., David Faure wrote: > > khtml/khtml_part.cpp, line 3580 > > <http://git.reviewboard.kde.org/r/111916/diff/2/?file=178128#file178128line3580> > > > > Is this variable still used? Yes. A couple of lines below - int n = readlink ( path.toLocal8Bit().data(), buff_two, 1022); I can replace it over here if you want? > On Aug. 12, 2013, 9:40 a.m., David Faure wrote: > > khtml/khtml_part.cpp, line 3585 > > <http://git.reviewboard.kde.org/r/111916/diff/2/?file=178128#file178128line3585> > > > > That's not what lstat does > > > > The case where stat() fails and lstat() succeeds is the case of a > > broken symlink. > > > > In that case, QFileInfo::exists() returns false. I guess all we can do > > then is skip this second check, and use if (info.isSymlink()) below, > > without ok&& in front. I.e. always go into the symlink code, whether the > > symlink is valid or broken. It's slightly confusing that a file can not exist, but can still be a system link. - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37565 --- On Aug. 12, 2013, 9:35 a.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111916/ > --- > > (Updated Aug. 12, 2013, 9:35 a.m.) > > > Review request for KDE Frameworks. > > > Description > --- > > Port khtml_part away from kde_file.h > > > Diffs > - > > khtml/khtml_part.cpp d944a29 > > Diff: http://git.reviewboard.kde.org/r/111916/diff/ > > > Testing > --- > > Compiles. The tests seem to segfault with and without this patch. I'll try to > diagnose it. > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Changes --- Fixed problems Description --- Port khtml_part away from kde_file.h Diffs (updated) - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111911: Port kioslave/ftp/ftp.cpp away from kde_file.h
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111911/
---
(Updated Aug. 12, 2013, 10:16 a.m.)
Review request for KDE Frameworks.
Changes
---
Use QFileInfo
Description
---
A couple of issues -
1. KDE::open/stat/etc take a QString and convert it to a char* via
QFile::encodeName(str).constData(). Qt obviously does not have methods to do
so. Instead of me doing it manually for each call to QT_OPEN/QT_STAT/etc, would
be it okay for me to declare local functions called KDE::stat/open?
Something along the lines of -
namespace KDE {
int open(const QString& filePath, ...) {
return QT_OPEN(QFile::encodeName(filePath).constData()), ...);
}
}
2. The kioslave uses KDE::utime to set the utime of file. I've used ::utime,
but that obviously won't work on non-unix platforms. What is the correct
solution?
One option is to add utime in qplatformdefs.h, but that is non trivial since Qt
seems to support about 104 different qplatformdefs and therefore all of them
will have to be updated.
Diffs (updated)
-
kioslave/ftp/ftp.cpp a0da54b
Diff: http://git.reviewboard.kde.org/r/111911/diff/
Testing
---
Doesn't even compile right now. With (1) it will start to compile.
Thanks,
Vishesh Handa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 19, 2013, 9:32 a.m.) Review request for KDE Frameworks. Changes --- Use QFileInfo::symLinkTarget() instead of relying on readlink. Description --- Port khtml_part away from kde_file.h Diffs (updated) - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112288: fillApplicationLanguages: Do not add the system locale
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112288/ --- (Updated Aug. 26, 2013, 1:06 p.m.) Review request for KDE Frameworks and Aleix Pol Gonzalez. Description --- If QLocale cannot find the appropriate language, it then defaults to the system locale. When adding all possible languages it is possible that QLocale::system() is added multiple times. This results in a huge list for the default language being added. For me, "US English" gets added over 50 times. Diffs - staging/xmlgui/src/kswitchlanguagedialog_p.cpp 894f2f4 Diff: http://git.reviewboard.kde.org/r/112288/diff/ Testing --- Ran kwindowtest and compared the list of languages shown in the switch languages dialog. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 112288: fillApplicationLanguages: Do not add the system locale
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112288/ --- Review request for KDE Frameworks and Aleix Pol Gonzalez. Description --- If QLocale cannot find the appropriate language, it then defaults to the system locale. When adding all possible languages it is possible that QLocale::system() is added multiple times. This results in a huge list for the default language being added. For me, "US English" gets added over 50 times. Diffs - staging/xmlgui/src/kswitchlanguagedialog_p.cpp 894f2f4 Diff: http://git.reviewboard.kde.org/r/112288/diff/ Testing --- Ran kwindowtest and compared the list of languages shown in the switch languages dialog. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112288: fillApplicationLanguages: Do not add the system locale
> On Aug. 26, 2013, 1:35 p.m., Kevin Ottens wrote:
> > staging/xmlgui/src/kswitchlanguagedialog_p.cpp, line 333
> > <http://git.reviewboard.kde.org/r/112288/diff/1/?file=184765#file184765line333>
> >
> > From QLocale documentation if the ctor didn't find the language in the
> > database it then uses ::default(). It just happens in your case that
> > ::system() == ::default(), but I think we should test for default() not
> > system().
> >
> > Now that means that your default language will never get in the combo
> > box... Don't you end up with an empty combo box with that patch? That would
> > be a problem.
> >
> > I wonder if we shouldn't store the default locale before the loop, set
> > C to be the new default and test on that instead. And last restore the
> > previous default language after the loop.
> >
> > It's jumping through hoops a bit but we're out of the usual QLocale
> > uses it seems.
>
> Albert Astals Cid wrote:
> To be honest I've been thinking about this code and I don't like it
> (Aleix and me did it together). Sure apply this patch, but a better way for
> us to find the instaled languages other than
>
> for ( int i = 2; i <= QLocale::LastLanguage; ++i )
>
> which doesn't work for us anyway since there's pt and pt_BR that is not
> covered here, basically my idea is to just iterate over
>
>QStandardPaths::standardLocations(QStandardPaths::GenericDataLocation)
> + QString::fromLatin1("locale/"); <-- note this does not compile :D
>
> (i.e. /usr/share/locale) to get the languages and then pass that to
> isApplicationTranslatedInto
>
> That should be muuuch better than the current loop we have, but don't
> prevent my suggestion for a better solution accept this if it is an
> improvement to the current one
@Kevin: One could also just do -
bool addedDefault = false;
for ( ... ) {
if (l == QLocale())
if (!addedDefault) {
addedDefault = true;
}
else
continue;
...
}
Or we could just add the default one in the beginning/end. Let me know which
you prefer. I've already implemented the default swapping one.
- Vishesh
-------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112288/#review38639
---
On Aug. 26, 2013, 1:06 p.m., Vishesh Handa wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112288/
> ---
>
> (Updated Aug. 26, 2013, 1:06 p.m.)
>
>
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
>
>
> Description
> ---
>
> If QLocale cannot find the appropriate language, it then defaults to the
> system locale. When adding all possible languages it is possible that
> QLocale::system() is added multiple times. This results in a huge list
> for the default language being added.
>
> For me, "US English" gets added over 50 times.
>
>
> Diffs
> -
>
> staging/xmlgui/src/kswitchlanguagedialog_p.cpp 894f2f4
>
> Diff: http://git.reviewboard.kde.org/r/112288/diff/
>
>
> Testing
> ---
>
> Ran kwindowtest and compared the list of languages shown in the switch
> languages dialog.
>
>
> Thanks,
>
> Vishesh Handa
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112288: fillApplicationLanguages: Do not add the system locale
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112288/ --- (Updated Aug. 26, 2013, 2:34 p.m.) Review request for KDE Frameworks and Aleix Pol Gonzalez. Changes --- Swap out the default with c Description --- If QLocale cannot find the appropriate language, it then defaults to the system locale. When adding all possible languages it is possible that QLocale::system() is added multiple times. This results in a huge list for the default language being added. For me, "US English" gets added over 50 times. Diffs (updated) - staging/xmlgui/src/kswitchlanguagedialog_p.cpp 894f2f4 Diff: http://git.reviewboard.kde.org/r/112288/diff/ Testing --- Ran kwindowtest and compared the list of languages shown in the switch languages dialog. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 112292: Make KLocalizedString::isApplicationTranslatedInto and QLocale::uiLanguages compatible
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112292/ --- Review request for KDE Frameworks, Albert Astals Cid and Aleix Pol Gonzalez. Description --- Make KLocalizedString::isApplicationTranslatedInto & QLocale::uiLanguages compatible QLocale::uiLanguages returns "lang-script-country" whereas KCatalog expects the format to be "lang_countr@script". We need to convert the format before checking if the corresponding catalog exists. Diffs - staging/ki18n/src/klocalizedstring.cpp eab9216 Diff: http://git.reviewboard.kde.org/r/112292/diff/ Testing --- Limited testing done. My QLocale::uiLanguages() returns "en-US" which is correctly converted to "en_US". I haven't tested it with anything else. Any tips on how I should go about testing this? Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112292: Make KLocalizedString::isApplicationTranslatedInto and QLocale::uiLanguages compatible
> On Aug. 26, 2013, 6:25 p.m., Albert Astals Cid wrote: > > if you want to see the language codes we have, they are listed in > > http://websvn.kde.org/trunk/l10n-kde4/ > > > > To be honest i don't see why we need conversion between uiLanguages and > > here since uiLanguages returns what we is set in LANGUAGE > > ./corelib/tools/qlocale_unix.cpp:231:case UILanguages: > > > > which we do set in KCatalog > > > > So basically should be returning "our stuff", no? > > > > It does return $LANGUAGE, but it changes the format by adding a '-' instead of '_'. Do you think qlocale_unix should be patched up instead? Because it is inconsistent with QLocale::name() which uses '_' So Qt code like this would fail - QLocale locale; locale.uiLanguages().contains(locale.name()); // < will return false! - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112292/#review38687 --- On Aug. 26, 2013, 2:48 p.m., Vishesh Handa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112292/ > --- > > (Updated Aug. 26, 2013, 2:48 p.m.) > > > Review request for KDE Frameworks, Albert Astals Cid, Aleix Pol Gonzalez, and > Chusslove Illich. > > > Description > --- > > Make KLocalizedString::isApplicationTranslatedInto & QLocale::uiLanguages > compatible > > QLocale::uiLanguages returns "lang-script-country" whereas KCatalog > expects the format to be "lang_countr@script". We need to convert the > format before checking if the corresponding catalog exists. > > > Diffs > - > > staging/ki18n/src/klocalizedstring.cpp eab9216 > > Diff: http://git.reviewboard.kde.org/r/112292/diff/ > > > Testing > --- > > Limited testing done. My QLocale::uiLanguages() returns "en-US" which is > correctly converted to "en_US". I haven't tested it with anything else. Any > tips on how I should go about testing this? > > > Thanks, > > Vishesh Handa > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 112303: KIO: Move krun tests to staging/kio
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112303/ --- Review request for KDE Frameworks. Description --- KIO: Move krun tests to staging/kio KRun has already been moved to staging/kio, but the tests were not moved Diffs - kio/tests/CMakeLists.txt 0d269a3 kio/tests/kruntest.h c897117 kio/tests/kruntest.cpp 5d419c1 kio/tests/krununittest.h 028be9d kio/tests/krununittest.cpp cd2a554 staging/kio/CMakeLists.txt aabf2c3 staging/kio/autotests/CMakeLists.txt 1b6f05e staging/kio/autotests/kiotesthelper.h PRE-CREATION staging/kio/autotests/krununittest.h PRE-CREATION staging/kio/autotests/krununittest.cpp PRE-CREATION staging/kio/tests/CMakeLists.txt PRE-CREATION staging/kio/tests/kruntest.h PRE-CREATION staging/kio/tests/kruntest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112303/diff/ Testing --- Compiled successfully + auto tests pass Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
What's up with KRun::runCommand?
Hey David
I was trying to port kmimetypechooser away from KRun, so I started to look at
the implementation of KRun. It seems to be somewhat strange -
bool KRun::runCommand(const QString& cmd, const QString &execName,
const QString & iconName,
QWidget* window, const QByteArray& asn, const QString&
workingDirectory)
{
//qDebug() << "runCommand " << cmd << "," << execName;
KProcess * proc = new KProcess;
proc->setShellCommand(cmd);
if (!workingDirectory.isEmpty()) {
proc->setWorkingDirectory(workingDirectory);
}
QString bin = binaryName(execName, true);
KService::Ptr service = KService::serviceByDesktopName(bin);
return runCommandInternal(cmd, service.data(),
execName /*executable to check for in
slotProcessExited*/,
execName /*user-visible name*/,
iconName, window, asn, workingDirectory);
}
It seems that the KProcess is created and then not used at all. The git log
doesn't seem to reveal much. Could you please take look?
--
Vishesh Handa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
