[cmake-developers] push of LinkOptionsCommand topic branch

Steve Wilson stevew at wolfram.com
Fri Feb 7 13:48:14 EST 2014


On Feb 7, 2014, at 1:45 AM, Stephen Kelly <steveire at gmail.com> wrote:

> You still have extra dashes in the titles in the target property 
> documentation.

Fixed.

> Please also rebase to master. The documentation has been updated to add more 
> relevant links, markup etc. Please follow the same patterns in all the new 
> docs on your branch. 

Updated

> Note also that your add_link_options doc copied some content from 
> add_compile_commands without modifying it (re include directories).

Fixed

> Here's some guidance Brad gave me a while ago regarding writing commit 
> messages with an imperative mood:
> 
> http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/6904

Thanks, these tips are quite helpful.

> The new tests look good to me. Please use spaces not tabs in foo.cpp in the 
> add_link_options test. You also add a foo.cpp in the target_link_options 
> test, but it has no content. Is that deliberate, or should it be the same as 
> the other?

In theory it doesn’t matter if foo.cpp is empty for the target_link_options test.   I went ahead and added some content though to match the add_link_options test, just to be consistent.

> In the 'cmLocalGenerator: Add AddLinkOptions method for LINK_FLAGS.' commit 
> message, you mention that the differences regarding static and object 
> libraries from the xcode generator are included. You don't say what impact 
> that has on other generators though. 
> 
> Were the other generators buggy by not doing the same thing before? 
> 
> Or was the xcode generator special for needing this? 
> 
> If the xcode generator has a special need, then that snippet should stay in 
> the xcode generator, right? 


Good questions.   The other generators did not specifically (that I noticed) have checks for static/object libraries.   It seems to me though that they maybe should have which is why I put the checks in AddLinkOptions.   If this decision should be changed though, I have no problem changing it.   I’m no expert on the generators.

I pushed the other changes back to stage.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140207/f566c03d/attachment-0002.sig>


More information about the cmake-developers mailing list