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.

Sign up for GitHub

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

Jump to bottom

Makefile and CMake style #871

Open
Frenzie opened this issue Mar 19, 2019 · 5 comments
Open

Makefile and CMake style #871

Frenzie opened this issue Mar 19, 2019 · 5 comments

Comments

@Frenzie
Copy link
Member

Frenzie commented Mar 19, 2019

I want to get rid of uppercase commands in the CMake files. It's currently a bit of a hodgepodge with a majority lowercase and I think it negatively affects maintainability (i.e., it takes me more time to scan than I think it should). Cf. https://community.kde.org/Policies/CMake_Coding_Style#Upper.2Flower_casing

Enforcement isn't completely self-evident but cmake-lint would probably do the job for some basic checks, also see https://gitlab.kitware.com/cmake/cmake/issues/17441

On a related note, https://github.com/mrtazz/checkmake exists to do similar things for Makefiles. There's also https://github.com/softprops/mint but at a glance that doesn't seem to do much other than checking if all targets exist.
@Frenzie
Copy link
Member Author

Frenzie commented Mar 21, 2019

First-pass added in #872.

Frenzie added a commit to Frenzie/koreader-base that referenced this issue Mar 21, 2019
@Frenzie
[chore] Makefile: declare all phonies
d9e9db7 
It's generally working as expected you're unlikely to have a file named `clean`. But if you were to create a file named `all` or `clean` you could have a pretty difficult time figuring out why nothing's happening anymore.

Cf. koreader/koreader#4819 and koreader#871.
@Frenzie Frenzie mentioned this issue Mar 21, 2019
Merged
Frenzie added a commit that referenced this issue Mar 21, 2019
@Frenzie
[chore] Makefile: declare all phonies (#875)
6789285 
It's generally working as expected you're unlikely to have a file named `clean`. But if you were to create a file named `all` or `clean` you could have a pretty difficult time figuring out why nothing's happening anymore.

Cf. koreader/koreader#4819 and #871.
Frenzie added a commit to Frenzie/koreader-base that referenced this issue Mar 21, 2019
@Frenzie
[chore] CMake kpvcrlib convert remaining tabs to four spaces
5eca7d6 
Cf. koreader#871.
@Frenzie Frenzie mentioned this issue Mar 21, 2019
Merged
Frenzie added a commit that referenced this issue Mar 21, 2019
@Frenzie
[chore] CMake kpvcrlib convert remaining tabs to four spaces (#877)
a0088f5 
Cf. #871.
@benoit-pierre
Copy link
Contributor

Is this still relevant?

@Frenzie
Copy link
Member Author

Frenzie commented May 29, 2024

I wouldn't mind having an autoformatter and/or linter for CMake like https://github.com/yamadapc/cmakefmt in place to keep things consistent.

Its output isn't dissimilar to how you reformatted things. But I'm also fine without.

@benoit-pierre
Copy link
Contributor

This might be more useful: https://github.com/cmake-lint/cmake-lint.

@Frenzie
Copy link
Member Author

Frenzie commented May 29, 2024

Yes, but I wasn't aware it had been forked and continued. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Frenzie @benoit-pierre