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

Feature/clang format #70

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Feature/clang format #70

wants to merge 5 commits into from

Conversation

isdampe
Copy link
Contributor

@isdampe isdampe commented Sep 11, 2022

This PR adds support for running clang-format across the code base to enforce the coding conventions developed earlier by @lachfoy.

Because clang-format will need to be manually installed on user systems, running the code formatter isn't required - it's just a way of tidying up code as developers deem necessary. Hopefully this is the best of both worlds - not introducing friction in environment setup for future new developers - but also giving a handy tool for anyone who wants to use it.

An entry in the Makefile has been added for running the formatter across the whole project - make format. Be warned - it will re-write files - so ensure you've committed or backed up files if you want to revert the styling.

And for proof of concept, I've run make format on the main branch. It does tidy the code base up nicely...

@lachfoy
Copy link
Contributor

lachfoy commented Sep 12, 2022

Which version of clang-format are you using? I've run into errors trying to run the format target:
image
The formatting that has been applied looks great, though!

@isdampe
Copy link
Contributor Author

isdampe commented Sep 13, 2022

Hey @lachfoy - I'm using clang-format version 14.0.6. Could you list the invalid argument errors here? We might get away with just removing them from the file.

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.

2 participants