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

[cmake] Reuse version.txt parsing across buildsystems #9742

Closed
wants to merge 1 commit into from

Conversation

hudokkow
Copy link
Member

@hudokkow hudokkow commented May 3, 2016

@hudokkow hudokkow added Type: Improvement non-breaking change which improves existing functionality CMake labels May 3, 2016
@wsnipex
Copy link
Member

wsnipex commented May 3, 2016

do we really need a new file for this? Whats wrong with macros?

@hudokkow
Copy link
Member Author

hudokkow commented May 3, 2016

Nope, no need for a new file.
Not unless we want to follow CMake's apparent philosophy of having one file per purpose. Kinda how upstreamed modules are coded and named.

Personally I would prefer one file per purpose but that's just me. It's easier to find what's doing what, easier to reuse, easier to maintain and easier to grasp the concept for a newcomer. Just compare scripts/common/ and modules/ readability. 😉
modules/ naming convention instantly conveys what each file code does.

On another note, my slight OCD finds annoying that we have *.cmake files with 3 naming conventions:

  • foobar
  • foo-bar
  • foo_bar

Can we agree on one? Perhaps inline with cmake? FooBar.cmake? Easier to read too...
Anyway, whatever we decide, now is the time to start enforcing it.

Oh, and nothing wrong with macros.

@wsnipex
Copy link
Member

wsnipex commented May 4, 2016

my personal preference would be to leave the version parsing as macro, since it is totally kodi specific. But if others vote for its own file I'll be ok with it.

As for file naming: Lets make it CamelCase like the Find modules.

@hudokkow hudokkow force-pushed the cmake_generate_version branch 4 times, most recently from c038861 to 1f1fb32 Compare May 18, 2016 15:22
@hudokkow
Copy link
Member Author

Closed in favour of #9845

@hudokkow hudokkow closed this May 20, 2016
@hudokkow hudokkow deleted the cmake_generate_version branch May 20, 2016 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants