-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmake namespace fixes #513
Conversation
For the consumer it should not matter if fmt has been added to the project as subdirectory or via find_package. With the alias targets the library can be always imported via fmt::fmt.
Seems reasonable. Thanks for the PR! |
Hi, what happened to this change? The alias is not defined in the current version. As a result
from the documentation http://fmtlib.net/latest/usage.html#header-only-usage-with-cmake yields a
|
It seems it was lost when the std branch was merged. In the 4.x branch it is still present. @vitaut would you mind to apply the patch again or should I send a new PR? |
Hmm, indeed looks like I missed it while merging. I'll reapply. |
On a second thought, I'm leaning towards killing the CMake namespace altogether (or, rather, not migrating it from the 4.x branch) because there are just two exported targets and both already have |
FWIW, I would keep the namespace. Like mentioned in the original PR #511, having the namespace is recommended by CMake's develop documentation and influences how errors are reported when something is wrong with the imported target. |
Good point, re-merged and added a comment why the namespace is useful. |
@niosHD, could you please review the updated CONTIBUTING document, particularly the part about licensing, and let me know if you agree with it being applied both to your contributions to {fmt}? The library is likely to be relicensed (#1073) so I'm collecting approval from all earlier contributors. |
Hi,
this PR addresses two small issues which have been missed in PR #511. First, the second target in the find-package-test has not been updated (see 9f48bb8). Second, in my opinion it should not matter how (
add_subdirectory
orfind_package
) the library has been added into a consuming project. I therefore added aliases with the prefix which makes it possible to always use the namespace version (fmt::fmt
) consistently (see a68c7a8).Regards,
niosHD