> 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.
I guess http://www.cmake.org/cmake/help/git-master/manual/cmake-developer.7.html#find-modules is the one to extend? > 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. Fair point, although for modules we've previously provided, I'm inclined to set them for ease of porting. > 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? 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... > 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. 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. > 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. Are you saying that INTERFACE_INCLUDE_DIRECTORIES line is wrong? Or that I should add a comment? > On Feb. 25, 2014, 3:56 p.m., Stephen Kelly wrote: > > docs/writing-find-modules.md, line 145 > > <https://git.reviewboard.kde.org/r/116025/diff/2/?file=246011#file246011line145> > > > > Actually it's mostly important so that users can set it in the cache if > > they have the library in a non-predicted location. Well, I meant the reason we set Foo_LIBRARY and then just set Foo_LIBRARIES to that same value, rather than using Foo_LIBRARIES directly. But I guess I should talk about making the cache entries the user has to set consistent. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116025/#review50830 ----------------------------------------------------------- On Feb. 25, 2014, 10:59 a.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, 10:59 a.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
