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

Distinguish "Greeter" as the name of the CMake project and as the name of files/directories/classes #82

Closed
nerrons opened this issue Feb 15, 2021 · 8 comments · Fixed by #84

Comments

@nerrons
Copy link
Contributor

nerrons commented Feb 15, 2021

Hi TheLartians

First of all thanks for this great project; it really simplifies things quite a bit.

One thing might be confusing for beginners though: the name "Greeter" is used both as the name of the CMake project and as the name of several files/directories/classes. Although you can technically distinguish these two by the capitalization of the first letter it still opens possibilities for mistakes.

One side effect in renaming the project name is also missing from the README: one needs to change #include <greeter/version.h> to #include <NewProjName/version.h> in .cpp files as well.

Do you think changing the project name to something like "YourProject" is better? I can make the PR.

@TheLartians
Copy link
Owner

Hey @nerrons, thanks for the suggestion! I do see how the naming can be confusing and lead to mistakes. I hoped that most problems should be easy to find thanks to the compiler's error message, e.g. after renaming a file. Tbh I'm not really sure how renaming the project to "YourProject" would help, as users would still run into the same issues?

@nerrons
Copy link
Contributor Author

nerrons commented Feb 16, 2021

Hey @nerrons, thanks for the suggestion! I do see how the naming can be confusing and lead to mistakes. I hoped that most problems should be easy to find thanks to the compiler's error message, e.g. after renaming a file. Tbh I'm not really sure how renaming the project to "YourProject" would help, as users would still run into the same issues?

Then they can fearlessly replace all YourProject to whatever they want without breaking anything. Now they probably still have to look at each occurrence and try to figure out what they mean.

@TheLartians
Copy link
Owner

Ah, so if I understand correctly, you suggest separating the CMake target name from the include destination? E.g. in CMake, the target would be called MyProject, but the include files would still be include/greeter/*.h?

@nerrons
Copy link
Contributor Author

nerrons commented Feb 16, 2021

Ah, so if I understand correctly, you suggest separating the CMake target name from the include destination? E.g. in CMake, the target would be called MyProject, but the include files would still be include/greeter/*.h?

Exactly! I hope that would clarify things a bit.

@TheLartians
Copy link
Owner

Yeah, I see how it would make iterative renaming easier. However, imo it's a good convention for libraries to use the project name in the include paths, which is kind-of enforced in the starter's structure. Removing it could lead to more derived projects changing both independently. Perhaps we can resolve the issue another way. Some ideas:

  1. Recommend using case-sensitive replace in the Readme and explain the naming scheme. Would be the easiest approach but requires users to read the readme carefully.
  2. Move all project-specific settings such as the project name and include path into a separate configuration file config.cmake. This would make it easier to get started with a new project, but would make the main CMakeLists less readable and discourage new users from learning / changing the actual CMake.
  3. Add a new self-removing command in cmake/tools.cmake that would allow to customise the project. E.g. after cloning you could run cmake -S. -Bbuild -DRENAME_CPP_STARTER=MyProject to automatically rename project and include paths.

Personally, I like both approaches 1. and 3.

@nerrons
Copy link
Contributor Author

nerrons commented Feb 16, 2021

I agree with your concerns about having include path names consistent with project names for libraries. Thanks for that point.

Regarding approach 3 it feels like we're doing too much, aren't we? We gonna have another target to maintain while what it accomplishes is just a few ⌃F's (or ⌘F's). It's also good for beginners to actually read before they start and at least go through the main CMakeLists.txt's at least once.

@TheLartians
Copy link
Owner

Agreed that 3 may be a bit overkill, so it seems 1 is the best option moving forward. If you want to go ahead and add the disclaimer to the Readme, I'd be happy to review the PR!

@nerrons
Copy link
Contributor Author

nerrons commented Feb 17, 2021

I changed the README a little bit in #84, please take a look 😄

@TheLartians TheLartians linked a pull request Feb 18, 2021 that will close this issue
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 a pull request may close this issue.

2 participants