Re: Review Request 110313: Some KUrifilter-plugin Krazy fixes

[Dawit Alemayehu]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110313/#review32045
---



kurifilter-plugins/ikws/ikwsopts.cpp


what exactly is  ? Is this i18n specific keyword ?



kurifilter-plugins/ikws/ikwsopts.cpp


What is the point of using a QPointer when the parent is set to NULL ?


- Dawit Alemayehu


On May 4, 2013, 10:50 p.m., Maarten De Meyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110313/
> ---
> 
> (Updated May 4, 2013, 10:50 p.m.)
> 
> 
> Review request for KDE Runtime.
> 
> 
> Description
> ---
> 
> This fixes most of the Krazy fixes for KUrifilter-plugins.
> http://www.englishbreakfastnetwork.org/krazy/reports/kde-4.x/kde-runtime/kurifilter-plugins/index.html
> 
> 
> Diffs
> -
> 
>   kurifilter-plugins/shorturi/kshorturifilter.cpp 802b325 
>   kurifilter-plugins/localdomain/localdomainurifilter.h 894b624 
>   kurifilter-plugins/ikws/searchproviderdlg.cpp eb4b31d 
>   kurifilter-plugins/ikws/searchprovider.h 7f26ba7 
>   kurifilter-plugins/ikws/ikwsopts_ui.ui 42e9675 
>   kurifilter-plugins/ikws/ikwsopts_p.h d048f08 
>   kurifilter-plugins/ikws/ikwsopts.cpp 8ca80b6 
>   kurifilter-plugins/fixhost/fixhosturifilter.h 8603e36 
>   kurifilter-plugins/tests/kurifiltertest.cpp dd18eba 
> 
> Diff: http://git.reviewboard.kde.org/r/110313/diff/
> 
> 
> Testing
> ---
> 
> Compiles.
> Only 4 issues remaining.
> 
> 
> Thanks,
> 
> Maarten De Meyer
> 
>

Re: Plasma Workspaces 4.11: the last feature release in the 4.xseries for kde-workspace

[Kevin Kofler]

On Friday 03 May 2013 at 17:04:44, Matthias Klumpp wrote:
> @Kevin: I am only remotely following this issue, but as PackageKit
> developer, I would of course like to see our project in Plasma
> Workspaces as soon as possible. But I don't know the exact issues
> here. Also, having KSecrets merged would be a nice goal too. (The
> SecretService API is very stable, and has the potential to become a
> new de-facto XDG standard soon)

The issues are the same in both cases: They're kdelibs features and kdelibs 
does not accept any features. By the time the first release of the Plasma 
Workspaces based on KF5 is planned, kdelibs will have been feature-frozen for 
THREE YEARS!!! (And I'm not even speaking of applications there!) This is a 
completely unacceptably long freeze period. It's time to reopen kdelibs for 
new features (though a lot of the damage has already been done! But still, 
it'd prevent any further damage) instead of freezing kde-workspace the same 
way.

> So, why not work on merging all this stuff into Workspaces 5 as early
> as possible, so it is present there?

Because Workspaces 5 / 2 / whatever (versioning and naming have become a total 
mess ever since the rebranding fiasco) is not anywhere near testable. Not even 
Frameworks 5 is (but my feature is not really testable without a working 
plasma-desktop anyway). At best I could dump a completely untested and 
untestable code drop.

In addition, even just compiling the change to verify that it even builds 
takes hours because there are no packages to work against. (We have Qt 5 
packages now going through review in Fedora, which is a lengthy process; KF5 
is not even started, and to be honest I'm not sure whether it even makes sense 
to start packaging it in its current stage.)

Testing my kdelibs 4.x patch was easy: I backported it to the version of 
kdelibs that was stable in Fedora, rebuilt my kdelibs package with it, updated 
kdelibs on my notebook, restarted plasma-desktop and tested the feature that 
way.

> (With having WS 4.11 frozen, it IMHO does not make any sense at all to
> develop new features for it, unless the new feature works on both
> Workspaces versions with less modifications)

The problem there is exactly that 4.x is frozen. It's not practical to develop 
features against 5 yet!

Kevin Kofler

Re: Plasma Workspaces 4.11: the last feature release in the 4.xseries for kde-workspace

[Kevin Kofler]

On Friday 03 May 2013 at 14:39:31, Aaron J. Seigo wrote:
> a completely workable approach was suggested. the work on ksecrets started
> and stopped a couple times.

A complicated workaround for the freeze was suggested that would have been a 
lot of work when the existing much simpler approach already worked. The 
approach would have had no technical advantage other than working around the 
pointless freeze. (Quite the opposite, the plugin approach that was suggested 
would have introduced a circular dependency in distribution packages.) It's no 
wonder the KSecrets developer didn't have the time and/or motivation to 
rewrite all his code for that approach.

> at one point it was tested for inclusion and apparently it did not work in
> some configurations (i forget the exact issue; it was quite a while ago and
> the discussion iirc was on kde-packager?). but usefully, it progressed quite
> far.

The version that got released didn't work at all:
* replacing KWallet didn't work because the kdelibs patch was rejected and the 
suggested plugin-based solution was never implemented,
* replacing gnome-keyring didn't actually work either, and the bug(s) which 
prevented that from working was/were never fixed because the project got 
abandoned due to the kdelibs freeze.

We tried packaging it in Fedora. The above was what we found out. And we were 
not alone: In fact, KSecrets got removed from later coordinated KDE releases 
due to it not working at all.

> hopefully you can put it in a repository that can be used by kdelibs which
> would both get around the 4.x kdelibs freeze *and* prepare it for
> frameworks.

I'm not the KSecrets developer.

Kevin Kofler

Re: Quotes around the application name in .desktop files

[David Faure]

On Wednesday 24 April 2013 09:37:16 Michael Spencer wrote:
> On Wednesday, April 24, 2013 09:21:57 AM David Faure wrote:
> > If you can test that removing the double-quotes works (including when
> > launching from KRun/klauncher, i.e. the standard way), even when the
> > caption has a space in it, then I'd be glad to see a patch removing all
> > double-quotes from .desktop files.
> 
> I have verified that the removing the double quotes works, even with a space
> in the caption, for KRun, the Application Launcher, and the Homerun
> Launcher, but I can't do the patch myself, as I'm new to KDE development
> and not familiar with patching in general.

Done, I removed double-quotes around %c in all the .desktop files I could find 
in my KDE SC checkouts (including extragear and playground).

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi

[Kevin Krammer]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/#review32015
---



libkfbapi/userinfo.h


No, this would be global symbol.

I meant as a constant of UserInfo, i.e. UserInfo::InvalidTimeZone.
It is a special value returned by UserInfo::timezone(), right?


- Kevin Krammer


On May 4, 2013, 9:34 a.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110083/
> ---
> 
> (Updated May 4, 2013, 9:34 a.m.)
> 
> 
> Review request for kdelibs and KDEPIM-Libraries.
> 
> 
> Description
> ---
> 
> kdepim-libs are needed in the facebook library only to return user info as 
> KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. 
> This patch makes dependency on kdepim-libs optional and disables building the 
> event classes completely in case kdepim-libs was not found.
> 
> 
> Diffs
> -
> 
>   LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION 
>   CMakeLists.txt 5aefdcf 
>   LibKFbAPI-KDEPIM.pc.in PRE-CREATION 
>   LibKFbAPI.pc.in af537d1 
>   libkfbapi/CMakeLists.txt dac62bc 
>   libkfbapi/commentinfo.h e5578c7 
>   libkfbapi/kdepim-utils.h PRE-CREATION 
>   libkfbapi/kdepim-utils.cpp PRE-CREATION 
>   libkfbapi/likeinfo.h e06052e 
>   libkfbapi/noteinfo.h 2492db1 
>   libkfbapi/noteinfo.cpp 154593d 
>   libkfbapi/notificationinfo.h a882694 
>   libkfbapi/userinfo.h ac22a7e 
>   libkfbapi/userinfo.cpp 26c64da 
>   libkfbapi/userinfoparser_p.h 0189a17 
> 
> Diff: http://git.reviewboard.kde.org/r/110083/diff/
> 
> 
> Testing
> ---
> 
> Builds correctly here in both cases
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

Re: NepomukCore - Do not merge KDE/4.10 into master

[Andreas Pakulat]

Hi,

On Sat, May 4, 2013 at 10:31 AM, Vishesh Handa  wrote:

> Hey everyone
>
> As you might have heard there was a fiasco in the nepomuk-core repository
> where the 'master' branch was accidentally merged into KDE/4.10. Since then
> the system admins had to do a hard reset to v4.10.2 and I had to manually
> cherry-pick a lot of the commits.
>
> I do not want anyone to merge KDE/4.10 into master. It will lead will a
> number of duplicate commits, and considering we already have a LOT of
> duplicates I do not want any more.
>

You have the duplicate commits anyway, merging the branch will not increase
their number. It also means you'll have to cherry-pick each bugfix done to
the KDE/4.10 branch manually into master, while merging gives you that for
free.

If the cherry-picked commits applied cleanly in KDE/4.10 when you
cherry-picked them, then merging KDE/4.10 into  master will not generate
conflicts.

Andreas

Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi

[Martin Klapetek]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/
---

(Updated May 4, 2013, 9:34 a.m.)


Review request for kdelibs and KDEPIM-Libraries.


Changes
---

Moved invalidTimezone constant to userinfo.h, so it can be reused


Description
---

kdepim-libs are needed in the facebook library only to return user info as 
KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. 
This patch makes dependency on kdepim-libs optional and disables building the 
event classes completely in case kdepim-libs was not found.


Diffs (updated)
-

  LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION 
  CMakeLists.txt 5aefdcf 
  LibKFbAPI-KDEPIM.pc.in PRE-CREATION 
  LibKFbAPI.pc.in af537d1 
  libkfbapi/CMakeLists.txt dac62bc 
  libkfbapi/commentinfo.h e5578c7 
  libkfbapi/kdepim-utils.h PRE-CREATION 
  libkfbapi/kdepim-utils.cpp PRE-CREATION 
  libkfbapi/likeinfo.h e06052e 
  libkfbapi/noteinfo.h 2492db1 
  libkfbapi/noteinfo.cpp 154593d 
  libkfbapi/notificationinfo.h a882694 
  libkfbapi/userinfo.h ac22a7e 
  libkfbapi/userinfo.cpp 26c64da 
  libkfbapi/userinfoparser_p.h 0189a17 

Diff: http://git.reviewboard.kde.org/r/110083/diff/


Testing
---

Builds correctly here in both cases


Thanks,

Martin Klapetek

NepomukCore - Do not merge KDE/4.10 into master

[Vishesh Handa]

Hey everyone

As you might have heard there was a fiasco in the nepomuk-core repository
where the 'master' branch was accidentally merged into KDE/4.10. Since then
the system admins had to do a hard reset to v4.10.2 and I had to manually
cherry-pick a lot of the commits.

I do not want anyone to merge KDE/4.10 into master. It will lead will a
number of duplicate commits, and considering we already have a LOT of
duplicates I do not want any more.

Thanks

-- 
Vishesh Handa

Re: Plasma Workspaces 4.11: the last feature release in the 4.xseries for kde-workspace

[Aaron J. Seigo]

On Friday, May 3, 2013 15:41:14 Michael Jansen wrote:
>   - Recommend forking them (gitorious, github, gko) for feature development

Where was this said .. *anywhere*? Git has great support for branches; you can 
also merge and 
separate repositories nearly at will; creating new repositories is easy .. in 
the case of the secret 
service implementation there is no need to fork anything, especially not on 
some 3rd party 
infrastructure.

> I somehow fail to grasp how recommending having n forks of those
> repositories (one for each patch/feature) around makes work for packagers
> and developers easier. 

I would also fail to grasp that recommendation.

Thankfully, it isn't what I suggested, nor have I heard anyone else who is 
actually involved in 
any of these efforts say that.

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.