[Okular-devel] Review Request 123249: Fixing the KActivities integration (Frameworks branch)

[Jan Kundrát]

> On April 9, 2015, 1:58 a.m., Jan Kundrát wrote:
> > -1 from me. The unconditional dep got introduced in 93918b1ec8f585dd135db1449e1b22b878f82fbc, which looks like a mistake to me. Why not:
> > -
> > -    find_package(KF5 CONFIG COMPONENTS Activities)
> 
> Ivan Čukić wrote:
>     While I don't mind making it again an optional dep, I want to mention that the Gwenview, Kate and KWrite made it a hard requirement. Is there a reason why Okular needs it to be optional?
> 
> Jan Kundrát wrote:
>     IMHO reducing dependencies is a good thing in general. I for one do not have a use for KActivities.
> 
> Ivan Čukić wrote:
>     If this is disabled during the build, the following will happen:
>     
>     - Kicker will not show documents opened with okular in the recent documents section
>     - Share plasmoid will not support sharing the currently opened document in okular
>     - Tasks applet will not show recent documents for okular
>     
>     ![Recent documents in Kicker](http://i.imgur.com/TVxCuL1.png)
>     ![Recent documents in Tasks](http://i.imgur.com/e4N38fQ.png)
>     ![Sharing a document](http://i.imgur.com/TVxCuL1.png)
> 
> Ivan Čukić wrote:
>     Duplicate image above
>     
>     ![Recent kickoff](http://i.imgur.com/r4VYCs2.png)

Anyway, if the Okular maintainers want to have this unconditional, that's their decision; I have plenty of RAM for those libs that I don't use :). What I was pointing out is just that the commit message (or actually the "text in this review") is not an accurate description of how these things actually happened. Maybe this would be a better description:

    Make KActivities a required dependency
    
    Our goal is better integration with the rest of the desktop (FIXME: just KDE or anybody else?).
    
    We have made a conscious decision to reduce our code a bit by removing the legacy #ifdefs. That way we do not have to support a build configuration which is going to be used by just a tiny minority of our users, and one that we are not terribly interested in. Without KActivities, there is for example no support for recording  and listing of recent documents, PDF sharing etc etc, and we honestly believe that the majority of our audience wants these features.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123249/#review78690
-----------------------------------------------------------


On April 3, 2015, 8:39 p.m., Ivan Čukić wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123249/
> -----------------------------------------------------------
> 
> (Updated April 3, 2015, 8:39 p.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> Since the a3fb02b881d commit, the KF5::Activities are a required dependency (CMakeLists.txt:31).
> 
> Still, the actual code uses ifdefs for KActivities_FOUND macro which no longer exists. This patch removes the macros and restores the feature.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 0a0f546 
>   shell/CMakeLists.txt ec36582 
>   shell/shell.h f345cf3 
>   shell/shell.cpp 6bb89c5 
> 
> Diff: https://git.reviewboard.kde.org/r/123249/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan Čukić
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20150425/216f88ae/attachment.html>