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

replace project name with ${PROJECT_NAME} #43

Closed
wants to merge 1 commit into from

Conversation

L0laapk3
Copy link

Should speak for itself

@ChrisThrasher
Copy link
Member

Repeat of #32

@L0laapk3
Copy link
Author

Whoops

@vittorioromeo
Copy link
Member

@ChrisThrasher can you provide the rejection rationale? I still think it's a worthwhile change.

@vittorioromeo vittorioromeo reopened this May 22, 2024
@eXpl0it3r
Copy link
Member

I believe it's the wrong approach as well, because we're mixing two things here, project and target.

Yes it works for this case, but we're teaching a wrong idea, that a project is target or a target is a project, which is not the case.

Maybe there's a middle ground of using a variable for the target name?

@vittorioromeo
Copy link
Member

I see, thanks for explaining, @eXpl0it3r.

but we're teaching a wrong idea, that a project is target or a target is a project, which is not the case

I'd argue we are making the same mistake even before this PR then :)

@Bromeon
Copy link
Member

Bromeon commented May 22, 2024

I'd argue we are making the same mistake even before this PR then :)

That was my thinking as well -- if those are two different things, would it not be clearer to name them differently (even if they happen to have the same value)?

@ChrisThrasher
Copy link
Member

ChrisThrasher commented May 22, 2024

can you provide the rejection rationale?

You're asking to write the CMake equivalent of this C++ code.

#define CAT_NAME Poe

struct Cat { int i; };

int main() {
    Cat CAT_NAME;
} 

In C++ when we want to reference something, we just spell out the variable name. We don't abstract that variable name behind a macro. To do the same in such a CMake project is way overkill and promotes a practice that scales very poorly to larger codebases.

I think the simplest solution is to rename the executable to main or something similarly terse. See #44.

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.

5 participants