----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.vidsolbach.de/r/297/#review287 -----------------------------------------------------------
Ship it! the patch looks ok. one question i have is whether or not this is 4.2 material; e.g. does it actually fix a bug or is this a feature enhacement? i think it's probably the latter? if so, this needs to wait for 4.3 to open up ... trunk/KDE/kdebase/workspace/plasma/containments/desktop/desktop.cpp <http://reviewboard.vidsolbach.de/r/297/#comment238> indentation =) also, why is this whole block wrapped in a ifndef? should just the max size bit do that? hmm.. actually, even better it should be using corona()->screenSize(screen()) and not Kephal directly here. trunk/KDE/kdebase/workspace/plasma/containments/panel/panel.cpp <http://reviewboard.vidsolbach.de/r/297/#comment239> hm. code duplication. that can't be good =) perhaps we should look at isolating all this code elsewhere ... i'm not sure *where* exactly yet, but in the shell is my first thought. not critical for this patch. trunk/KDE/kdelibs/plasma/corona.h <http://reviewboard.vidsolbach.de/r/297/#comment240> freeScreenEdges(int screen)? perhaps a bit more readable that way? - Aaron On 2008-12-10 03:30:38, Alessandro Diaferia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.vidsolbach.de/r/297/ > ----------------------------------------------------------- > > (Updated 2008-12-10 03:30:38) > > > Review request for Plasma. > > > Summary > ------- > > This patch tries to place correctly the panel filling the free edges in the > current screen. > > > Diffs > ----- > > trunk/KDE/kdebase/workspace/plasma/containments/desktop/CMakeLists.txt > trunk/KDE/kdebase/workspace/plasma/containments/desktop/desktop.cpp > trunk/KDE/kdebase/workspace/plasma/containments/panel/panel.cpp > trunk/KDE/kdelibs/plasma/corona.h > trunk/KDE/kdelibs/plasma/corona.cpp > > Diff: http://reviewboard.vidsolbach.de/r/297/diff > > > Testing > ------- > > > Thanks, > > Alessandro > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
