> On Feb. 25, 2014, 3:56 p.m., Stephen Kelly wrote: > > docs/writing-find-modules.md, line 9 > > <https://git.reviewboard.kde.org/r/116025/diff/2/?file=246011#file246011line9> > > > > You can link to > > > > http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html > > > > for upstream info on this. > > Alex Merry wrote: > I guess > http://www.cmake.org/cmake/help/git-master/manual/cmake-developer.7.html#find-modules > is the one to extend?
Yep. > On Feb. 25, 2014, 3:56 p.m., Stephen Kelly wrote: > > docs/writing-find-modules.md, line 50 > > <https://git.reviewboard.kde.org/r/116025/diff/2/?file=246011#file246011line50> > > > > The imported targets should be the primary way of using the module. I > > don't think the _INCLUDES and _LIBRARIES etc variables should even be set > > if imported targets are provided. > > Alex Merry wrote: > Fair point, although for modules we've previously provided, I'm inclined > to set them for ease of porting. include_directories(${Foo_INCLUDES}) should be harmless if Foo_INCLUDES is not set. > On Feb. 25, 2014, 3:56 p.m., Stephen Kelly wrote: > > docs/writing-find-modules.md, line 98 > > <https://git.reviewboard.kde.org/r/116025/diff/2/?file=246011#file246011line98> > > > > I don't think AUTHOR_WARNING is appropriate here. Why not WARNING? > > Alex Merry wrote: > Because it's a warning for authors (of project build scripts) rather than > users (ie: those building the package). I thought that was exactly what > AUTHOR_WARNING was for... Perhaps. ecm is different because the maintainer/author of the Find modules is not the one using it, which is generally the case for third party find modules. Anyway, the cmake-shipped Find-modules also use AUTHOR_WARNING. So, there's precedent, whether it's the right or wrong thing to do. > On Feb. 25, 2014, 3:56 p.m., Stephen Kelly wrote: > > docs/writing-find-modules.md, line 153 > > <https://git.reviewboard.kde.org/r/116025/diff/2/?file=246011#file246011line153> > > > > Don't set Foo_VERSION_STRING, set Foo_VERSION. That is canonical > > because of config-file packages. > > Alex Merry wrote: > OK; I was following a pattern from other CMake scripts. Also, it is what > http://www.cmake.org/cmake/help/git-master/manual/cmake-developer.7.html#find-modules > suggests. The behavior of the config-files determines what is canonical. If the documentation doesn't recommend the canonical variable name, then I argue the documentation is incorrect. > On Feb. 25, 2014, 3:56 p.m., Stephen Kelly wrote: > > docs/writing-find-modules.md, line 325 > > <https://git.reviewboard.kde.org/r/116025/diff/2/?file=246011#file246011line325> > > > > Note that only build-properties of the target itself should be here. > > Not those of dependencies (if the dependency provides imported targets, > > which it should/must for this stuff to work). CMake will resolve that > > itself. > > Alex Merry wrote: > Are you saying that INTERFACE_INCLUDE_DIRECTORIES line is wrong? Or that > I should add a comment? Perhaps add a comment. The INTERFACE_INCLUDE_DIRECTORIES line seems correct. - Stephen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116025/#review50830 ----------------------------------------------------------- On Feb. 25, 2014, 8:11 p.m., Alex Merry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116025/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2014, 8:11 p.m.) > > > Review request for Build System, Extra Cmake Modules and KDE Frameworks. > > > Repository: extra-cmake-modules > > > Description > ------- > > Add documentation about writing find modules > > > Diffs > ----- > > README.md 85b97b7fa003282e1eeb1113c4668a9b73e3f731 > docs/writing-find-modules.md PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/116025/diff/ > > > Testing > ------- > > > Thanks, > > Alex Merry > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
