Skip to content

Use clang-format to enforce code conventions #1012

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

Closed
arturo182 opened this issue Jul 10, 2018 · 7 comments
Closed

Use clang-format to enforce code conventions #1012

arturo182 opened this issue Jul 10, 2018 · 7 comments

Comments

@arturo182
Copy link
Collaborator

As @tannewt mentioned in another issue, this would make sure that all code is uniformly formatted.
So let's have a discussion about how to implement this and which code style to use.

For the implementation we should run it as a part of the Travis CI for each PR for sure, but is there a way to run it also on users' computers before they commit. It can be done with pre-commit git hooks but those need to be setup manually by each user which is not great.

As for the code style, we can either use a predefined format or make our own, more info here: https://clang.llvm.org/docs/ClangFormatStyleOptions.html

Thirdly, the Code Conventions link in https://github.com/adafruit/circuitpython/blob/master/CONTRIBUTING.md links to the micropython file, as it seems we don't have one in our repository, should we copy it locally, change the link and eventually modify the style to match our decisions?

And lastly, to what extent should the clang-format be applied, don't think we can expect all of the C code to be properly formatted, should it only be shared-bindings, shared-modules and the nrf and samd ports, or more?

@dhalbert
Copy link
Collaborator

My major worry is making sure it's easy to continue to merge upstream micropython/micropython. It might be necessary to clang-format the upstream code before the merge; up to now our coding style has matched upstream and we haven't had to do something like that.

My major requirements are four-space indents, an opening curly brace does not stand alone, and always use curly braces in control structures. These are already what we do. For everything else I don't care very much, and don't actually think there needs to be an enforced style. For instance, how to indent arguments on multiple lines is not a hot button for me, nor line length, nor comment indentation or alignment. For case statements I use the standard style Emacs provides (which seems to match the current code), but I don't have a strong feeling about that.

@dhalbert
Copy link
Collaborator

I went over the micropython conventions, and I'm completely happy with them (e.g. // not /* */ comments)

@arturo182
Copy link
Collaborator Author

I agree on priorities with @dhalbert , I think agreeing on how switch/case/break is aligned would be nice as well :)

@tannewt
Copy link
Member

tannewt commented Jul 10, 2018

I started experimenting with this here: https://github.com/tannewt/circuitpython/tree/clang-format

Merging should be easy as long as we stick with choices that can be fixed programmatically. (This is the goal anyway so we no longer need to worry about style.)

We'll need to pick a specific version of clang-format that is available on Travis.

@tannewt tannewt added this to the Long term milestone Oct 9, 2018
@dhalbert
Copy link
Collaborator

MicroPython has done this: micropython#4223

@tannewt
Copy link
Member

tannewt commented Jul 21, 2020

I don't think that MicroPython is using clang-format. They are using astyle IIRC.

@microdev1
Copy link
Collaborator

Closing... code formatting using Uncrustify for c and Black for python implemented in #4362.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants