[Kde-graphics-devel] Review Request 122737: Re-origanise code in the frameworks branch

[Boudhayan Gupta]

> On Feb. 27, 2015, 5:09 p.m., Aaron J. Seigo wrote:
> > Too many things in one patch.
> > 
> > Until the frameworks branch is merged into master, I don't want files moving around unless there is a *compelling* reason for it, and in this case there isn't.
> > 
> > Moving the version definition to the cmake file is fine; adding guards to the generated header is also fine.

Thanks for the feedback.

I can revert the moves, and submit a new patch with just moving version definition to CMakeLists and guarding the headers. About the re-factored CMakeLists - should I make only minimal changes (just adding the version definitions), or tidy it up so that if one day the files move around, creating the new CMakeLists for the subdirs is just a matter of cutting out bits from the top level file and pasting them into the subdir ones?


- Boudhayan


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


On Feb. 27, 2015, 1:24 p.m., Boudhayan Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122737/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 1:24 p.m.)
> 
> 
> Review request for KSnapshot and Aleix Pol Gonzalez.
> 
> 
> Repository: ksnapshot
> 
> 
> Description
> -------
> 
> This is a pretty big patch. It does the following:
> 
> * Move all the .cpp and .h files to an "src" directory
> * Move the XDG desktop file to a "desktop" directory
> * Move the SVG icon source for the hicolor icon to the icons directory
> * Refactor the CMakeLists.txt file
> * Give each subdirectory (src, icons, desktop) its own CMakeLists.txt. doc already had one.
> * Guard config-ksnapshot.h.cmake contents within ifndef-define-endif preprocessor directives
> * Move the KSNAPVERSION define from main.cpp to config-ksnapshot.h.cmake
> * The KSnapshot version is now defined in the top-level CMakeLists.txt
> 
> 
> Diffs
> -----
> 
>   src/platforms/xcb/pixmaphelper.h PRE-CREATION 
>   src/platforms/xcb/pixmaphelper.cpp PRE-CREATION 
>   src/platforms/xcb/windowgrabber.cpp PRE-CREATION 
>   src/platforms/xcb/xcbutils.h PRE-CREATION 
>   src/regiongrabber.h PRE-CREATION 
>   src/regiongrabber.cpp PRE-CREATION 
>   src/snapshottimer.h PRE-CREATION 
>   src/snapshottimer.cpp PRE-CREATION 
>   src/windowgrabber.h PRE-CREATION 
>   src/windowgrabber.cpp PRE-CREATION 
>   windowgrabber.h f8c0b17 
>   windowgrabber.cpp 4672679 
>   src/ksnapshotwidget.ui PRE-CREATION 
>   src/main.cpp PRE-CREATION 
>   src/org.kde.ksnapshot.xml PRE-CREATION 
>   src/platforms/windows/windowgrabber.cpp PRE-CREATION 
>   src/ksnapshot.cpp PRE-CREATION 
>   src/ksnapshot_options.h PRE-CREATION 
>   src/ksnapshotimagecollectionshared.h PRE-CREATION 
>   src/ksnapshotimagecollectionshared.cpp PRE-CREATION 
>   src/ksnapshotinfoshared.h PRE-CREATION 
>   src/ksnapshotinfoshared.cpp PRE-CREATION 
>   src/ksnapshotobject.h PRE-CREATION 
>   src/ksnapshotobject.cpp PRE-CREATION 
>   src/ksnapshotpreview.h PRE-CREATION 
>   src/ksnapshotpreview.cpp PRE-CREATION 
>   src/ksnapshotsendtoactions.h PRE-CREATION 
>   src/ksnapshotsendtoactions.cpp PRE-CREATION 
>   main.cpp ef03554 
>   org.kde.ksnapshot.desktop fdc8831 
>   org.kde.ksnapshot.xml eb1d8e0 
>   platforms/windows/windowgrabber.cpp 8cf1225 
>   platforms/xcb/pixmaphelper.h e3f5695 
>   platforms/xcb/pixmaphelper.cpp 92fb91d 
>   platforms/xcb/windowgrabber.cpp 5de9473 
>   platforms/xcb/xcbutils.h b57b692 
>   regiongrabber.h 2cf9c40 
>   regiongrabber.cpp 97af3fc 
>   snapshottimer.h 93dc15c 
>   snapshottimer.cpp 5cc3de2 
>   src/CMakeLists.txt PRE-CREATION 
>   src/Messages-i18n.sh PRE-CREATION 
>   src/Messages-qt.sh PRE-CREATION 
>   src/config-ksnapshot.h.cmake PRE-CREATION 
>   src/expblur.cpp PRE-CREATION 
>   src/freeregiongrabber.h PRE-CREATION 
>   src/freeregiongrabber.cpp PRE-CREATION 
>   src/kbackgroundsnapshot.h PRE-CREATION 
>   src/kbackgroundsnapshot.cpp PRE-CREATION 
>   src/kipiimagecollectionselector.h PRE-CREATION 
>   src/kipiimagecollectionselector.cpp PRE-CREATION 
>   src/kipiinterface.h PRE-CREATION 
>   src/kipiinterface.cpp PRE-CREATION 
>   src/ksnapshot.h PRE-CREATION 
>   ksnapshotpreview.cpp 50bda19 
>   ksnapshotsendtoactions.h f0b4f8f 
>   ksnapshotsendtoactions.cpp a8c4ccb 
>   ksnapshotwidget.ui c9b4258 
>   CMakeLists.txt d174abf 
>   Messages.sh d242e38 
>   config-ksnapshot.h.cmake 3514dd5 
>   desktop/CMakeLists.txt PRE-CREATION 
>   desktop/org.kde.ksnapshot.desktop PRE-CREATION 
>   expblur.cpp 173b1d7 
>   freeregiongrabber.h 4f8e695 
>   freeregiongrabber.cpp c60b0ca 
>   hisc-app-ksnapshot.svgz 33ab303 
>   icons/CMakeLists.txt PRE-CREATION 
>   icons/hisc-app-ksnapshot.svgz PRE-CREATION 
>   kbackgroundsnapshot.h 7289e63 
>   kbackgroundsnapshot.cpp 8aefbb3 
>   kipiimagecollectionselector.h 03a13c3 
>   kipiimagecollectionselector.cpp 78c2ca6 
>   kipiinterface.h 8d866ef 
>   kipiinterface.cpp 4d32647 
>   ksnapshot.h 71ecd6f 
>   ksnapshot.cpp 55e93ee 
>   ksnapshot_options.h 36ab496 
>   ksnapshotimagecollectionshared.h 6cc6d9b 
>   ksnapshotimagecollectionshared.cpp da097f0 
>   ksnapshotinfoshared.h b69423d 
>   ksnapshotinfoshared.cpp 5892adb 
>   ksnapshotobject.h a69253c 
>   ksnapshotobject.cpp 06a8546 
>   ksnapshotpreview.h 83ddf55 
> 
> Diff: https://git.reviewboard.kde.org/r/122737/diff/
> 
> 
> Testing
> -------
> 
> Builds and installs fine. No functionality was added in this patch, so didn't test much beyond build/run/take screenshots.
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-graphics-devel/attachments/20150227/6f37c373/attachment.html>