Review Request 119929: Cleaning up plasma-shell, episode 1

[Aaron J. Seigo]

> On Aug. 25, 2014, 1:51 p.m., Kai Uwe Broulik wrote:
> >
> 
> Aleix Pol Gonzalez wrote:
>     Can we maybe run a script or something after this review? It's already hard enough to decode the patch...
> 
> Marco Martin wrote:
>     maybe after this is merged could be tried to run astyle from
>     https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Artistic_Style_.28astyle.29_automatic_code_formatting
>     ?

I actually tried astyle but the command line options have changed yet again and the various incantations I tried made more of a mess as it made more changes than desired. If someone else can make it "go" though, that'd be great as I would very much appreciate a more consistent code base to work off of.


- Aaron J.


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


On Aug. 25, 2014, 2:50 p.m., Aaron J. Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119929/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2014, 2:50 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> While reviewing the plasma-shell binary for use in a device project, I came across a number of issues which are fixed in a longer-than-probably-desirable series of patches in the clean_shellcorona branch of plasma-workspace. There are two types of changes:
> 
> * improving multiscreen code; in particular there was a recursive function that would (I assume innadvertently) migrate panels to the last screen when detached. there was also potentially unecessary signal emissions, 
> * code cleanups
> 
> The code cleanups come in a few flavors:
> * getting code in line with the kdelibs coding style this project purportedly follows
> * removal of dead code
> * checking of parameters where sensible
> * making debug code only available in debug builds (c.f. CHECK_SCREEN_INVARIANTS; this would previously result in a bunch of compiler notices when built in release mode)
> 
> It is probably easier to review the patches in the branch than the monster review this has become, however many of the patches (though admittedly not all) are built one atop the other making individual review tricky; it was one of those "tug on a loose thread, and the whole sweater came unravelled" type things.
> 
> 
> Diffs
> -----
> 
>   shell/scripting/scriptengine.cpp 94e93fe810edb1743b84cdca168bc7f7e556e982 
>   shell/shellcorona.h ebfbdc7ceea9f6cfb387c1931e78faa9e43a894d 
>   shell/shellcorona.cpp 29935a51ffc490e1d7e91e2a1109764a4ff2d101 
>   shell/widgetexplorer/widgetexplorer.cpp 0ad5cc8519c8f37ce20e49332491ea0469ee7a29 
>   shell/containmentconfigview.cpp f489f1771f3de0a0a5fb7e385615e5981b2f240f 
>   shell/desktopview.cpp 0f8bde7af9bae1a17ff42d54c9882fbddc695694 
> 
> Diff: https://git.reviewboard.kde.org/r/119929/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140825/b43c44fe/attachment-0001.html>