Review Request 119267: Adding KWindowSystem::setOnActivities(WId win, const QStringList &activities) method

Ivan Čukić ivan.cukic at kde.org
Thu Aug 7 05:15:29 UTC 2014



> On Aug. 6, 2014, 5:04 p.m., Thomas Lübking wrote:
> > src/netwm.cpp, line 4779
> > <https://git.reviewboard.kde.org/r/119267/diff/3/?file=296945#file296945line4779>
> >
> >     -> qstrdup?
> >     (also "new code" issue)

nstrdup is used all over the file, qstrdup not. Would it be wise to start using a new function just in a single place?


> On Aug. 6, 2014, 5:04 p.m., Thomas Lübking wrote:
> > src/netwm.cpp, line 4784
> > <https://git.reviewboard.kde.org/r/119267/diff/3/?file=296945#file296945line4784>
> >
> >     C style cast.
> >     
> >     While i personally don't mind that at all (and it's used in the file anyway) there are objections against it and Martin might disapprove adding more.

It works even without the cast. I've removed it.


- Ivan


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


On Aug. 5, 2014, 8:21 p.m., Ivan Čukić wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119267/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 8:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> Currently, the library only has the method for retrieving a list of activities a window belongs to.
> 
> This is adding a method which provides changing the list of activities for a window.
> 
> 
> Diffs
> -----
> 
>   autotests/kwindowinfox11test.cpp 50ce806 
>   src/kwindowinfo_x11.cpp 041dfd3 
>   src/kwindowsystem.h 0b58e71 
>   src/kwindowsystem.cpp fb59603 
>   src/kwindowsystem_p.h 8861844 
>   src/kwindowsystem_p_x11.h 9baa6ae 
>   src/kwindowsystem_x11.cpp 2016820 
>   src/netwm.h 2d812a7 
>   src/netwm.cpp 1daad1e 
>   src/netwm_p.h a201cb6 
> 
> Diff: https://git.reviewboard.kde.org/r/119267/diff/
> 
> 
> Testing
> -------
> 
> Yes, works with the new activity switcher.
> 
> 
> Thanks,
> 
> Ivan Čukić
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140807/b698806b/attachment.html>


More information about the Kde-frameworks-devel mailing list