Skip to content
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

- move cmake_find_package_multi generator to the top #2269

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Oct 21, 2021

for newbies, it's not obvious what should be the reasonable default choice for the cmake generator amongst others:

  • cmake
  • cmake_multi
  • cmake_paths
  • cmake_find_package
  • cmake_find_package_multi
  • CMakeDeps

as we expect (most of) readers to read the documentation from the top to the bottom, IMO it's reasonable to place a default choice to the top rather to the very bottom.

Signed-off-by: SSE4 <tomskside@gmail.com>
@SSE4 SSE4 requested a review from jgsogo October 21, 2021 07:34
@jgsogo jgsogo requested a review from lasote October 21, 2021 17:19
@jgsogo jgsogo assigned czoido and unassigned jgsogo Oct 21, 2021
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would refactor the list completely, something like:

Recommended generators:
 * CMakeDeps (default in Conan v2)
 * cmake (to be deprecated in v2)
 * cmake_find_package_multi (to be deprecated in v2)
 * cmake_find_package (to be deprecated in v2)
 * ...
 
Not recommended (already deprecated):
 * cmake_paths
 * cmakemulti

wdyt, @lasote @czoido ?

(And same comments for #2268)

@czoido
Copy link
Contributor

czoido commented Oct 21, 2021

Yes, I agree with marking the current state and the plans for the future. In fact, in my opinion, I would only recommend CMakeDeps that is going to be default for the examples in the documentation...

@lasote
Copy link
Contributor

lasote commented Oct 22, 2021

Agree with @czoido

@SSE4
Copy link
Contributor Author

SSE4 commented Oct 22, 2021

so, to clarify, it should look like:

Recommended generators:
 * CMakeDeps (default in Conan v2)

Not recommended (already deprecated):
 * cmake (to be deprecated in v2)
 * cmake_find_package_multi (to be deprecated in v2)
 * cmake_find_package (to be deprecated in v2)
 * cmake_paths (to be deprecated in v2)
 * cmake_multi (to be deprecated in v2)

@danimtb
Copy link
Member

danimtb commented Oct 22, 2021

so, to clarify, it should look like:

Recommended generators:
 * CMakeDeps (default in Conan v2)

Not recommended (already deprecated):
 * cmake (to be deprecated in v2)
 * cmake_find_package_multi (to be deprecated in v2)
 * cmake_find_package (to be deprecated in v2)
 * cmake_paths (to be deprecated in v2)
 * cmake_multi (to be deprecated in v2)

I wouldn't say that cmake_find_package/_multi are generators already deprecated, just not recommended over CMakeDeps

@SSE4
Copy link
Contributor Author

SSE4 commented Oct 22, 2021

I wouldn't say that cmake_find_package/_multi are generators already deprecated, just not recommended over CMakeDeps

Recommended generators:
 * CMakeDeps (default in Conan v2)

Not recommended (to be deprecated in v2):
 * cmake_find_package_multi
 * cmake_find_package

Not recommended (already deprecated):
 * cmake 
 * cmake_paths
 * cmake_multi

is it good enough?

@jgsogo
Copy link
Contributor

jgsogo commented Oct 22, 2021

My only concern is that, if we are to recommend CMakeDeps, it can't have this warning at the top... we can't recommend breaking changes.

image

@lasote
Copy link
Contributor

lasote commented Oct 22, 2021

But we recommend it... why we cant?

@SSE4
Copy link
Contributor Author

SSE4 commented Oct 22, 2021

I don't see any problem with that - we use and recommends many things that are experimental: cppstd, self.cpp_info.components, hooks, lockfiles, two profiles, etc.

even cmake_find_package_multi is marked as experimental...

image

seems like in that aspect, there is no difference between CMakeDeps and cmake_find_package_multi.

@jgsogo
Copy link
Contributor

jgsogo commented Oct 22, 2021

We use things that are experimental, yes.

We recommend things that are experimental, yes... but we do it in issues, PRs, and slack.

We don't write side by side in the documentation: "A will have breaking changes. We recommend you to use A. Any alternative to A is deprecated". IMHO it is not the best message you can send from docs. It leaves you with no alternative if you are considering Conan for anything serious 🤷 . People reading docs (1M page views per month) don't read issues, PRs, or slack.

@SSE4
Copy link
Contributor Author

SSE4 commented Oct 22, 2021

We don't write side by side in the documentation: "A will have breaking changes. We recommend you to use A. Any alternative to A is deprecated". IMHO it is not the best message you can send from docs. It leaves you with no alternative if you are considering Conan for anything serious 🤷 . People reading docs (1M page views per month) don't read issues, PRs, or slack.

if we talk about sources of truths where we recommend things, it's not just docs, but also:

  • conan-center itself, and recipes there are good practices from where people copy and learn
  • all our demos and presentations we do at CppCon, Italian C++ or whatever else
  • slack, issues, stackoverflow, twitter, reddit, etc. ofcourse, but not so significant as things above

but my main point is that CMakeDeps and cmake_find_package_multi are both marked as experimental in the current docs, so they are eqaul in that aspect. if you say we can't recommend CMakeDeps in docs, it also means we can't recomment cmake_find_package_multi in docs.

@jgsogo
Copy link
Contributor

jgsogo commented Oct 22, 2021

but my main point is that CMakeDeps and cmake_find_package_multi are both marked as experimental in the current docs, so they are eqaul in that aspect. if you say we can't recommend CMakeDeps in docs, it also means we can't recomment cmake_find_package_multi in docs.

Absolutely. I only think docs should be aligned with our message, and our message should be supported by the docs.

@prince-chrismc
Copy link
Contributor

I think I can finally close this with #2878 as it's will have updated the language around both the old and new generators 🤞

@Croydon
Copy link
Contributor

Croydon commented Feb 26, 2023

so, to clarify, it should look like:

Recommended generators:
 * CMakeDeps (default in Conan v2)

Not recommended (already deprecated):
 * cmake (to be deprecated in v2)
 * cmake_find_package_multi (to be deprecated in v2)
 * cmake_find_package (to be deprecated in v2)
 * cmake_paths (to be deprecated in v2)
 * cmake_multi (to be deprecated in v2)

I think this is still the best idea as it gives you a really quick overview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants