On 06/21/2016 05:50 PM, Fabiano Fidêncio wrote: > On Tue, Jun 21, 2016 at 10:39 PM, Eduardo Lima (Etrunko) > <etrunko@xxxxxxxxxx> wrote: >> On 06/21/2016 05:04 PM, Fabiano Fidêncio wrote: >>> GtkRevealer was intrudced in Gtk+ 3.10 and, combined with Gtk Overlay >>> (intoduced in Gtk+ 3.2), can provide a more sustainably implementation >>> of the AutoDrawer functionality. >>> >>> This approach is completely based on the approach taken by virt-manager: >>> https://github.com/virt-manager/virt-manager/commit/dc05600324f6b9a82b68581fc0a9c145f9889ce9 >>> >>> Resolves: https://bugs.freedesktop.org/show_bug.cgi?id=94495 >>> >>> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx> >>> --- >>> src/Makefile.am | 8 +- >>> src/resources/ui/virt-viewer.ui | 401 +++++++------- >>> src/view/autoDrawer.c | 991 ---------------------------------- >>> src/view/autoDrawer.h | 91 ---- >>> src/view/drawer.c | 366 ------------- >>> src/view/drawer.h | 83 --- >>> src/view/ovBox.c | 946 -------------------------------- >>> src/view/ovBox.h | 103 ---- >>> src/view/virt-viewer-timed-revealer.c | 224 ++++++++ >>> src/view/virt-viewer-timed-revealer.h | 74 +++ >>> src/virt-viewer-window.c | 36 +- >>> 11 files changed, 521 insertions(+), 2802 deletions(-) >> >> The joys of cleaning up code. I have some general questions before going >> deep on the review. >> >> 1) Do you think the new files could go under src/ instead of src/view/ >> directory? Those seemed to be used only for the old widget. > > Well, I personally don't have a strong opinion about where those files > are placed. Just left there because the files used for the old widget > were there. Yeah, if you want my suggestion, get rid of that directory. :P > >> >> 2) Even better, it seems that timed-revealed has more boilerplate code >> than code that really does something useful. Any specific reason why >> those functions could not be part of virt-viewer-window.c ? > > Just because I preferred to keep the implementation details as close > as possible to the one used in virt-manager. The functions could be > moved to the virt-viewer-window.c, but I, particularly, fail to see a > good reason for doing this. Actually, IMHO, it seems more organized > the way it's now than moving the functions to the virt-viewer-window.c > file. Okay, I just read the code, I like it the way it is. I have posted a few comments about it. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etrunko@xxxxxxxxxx _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list