-
Notifications
You must be signed in to change notification settings - Fork 177
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
Formatted C files #416
Formatted C files #416
Conversation
Thanks for looking into this! I haven't reviewed it in any detail yet, but from the CI, it looks like |
I ran the tests on my machine and had no errors. I'll look into it. |
Nailed it. I think I messed with |
😱 <- that should be a reaction option :) |
It looks pretty good.
Two things jump out:
|
Thanks for adjusting the rules. The C files look a lot better than their state before this PR, IMO. I believe two things remain:
|
src/unix/lwt_unix.h
Outdated
#include <caml/socketaddr.h> | ||
#include <caml/unixsupport.h> | ||
#include <lwt_config.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh, this build failure suggests that either caml/unixsupport.h
or lwt_config.h
has to manually be moved above caml/socketaddr.h
. This is an upstream issue due to one of the headers not including the other one internally, but we have to work around it :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A trick is to have separate "paragraphs", clang-format will order includes by-paragraph. I've amended the commit. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'm learning :)
Much appreciated. I find these files much easier to read now, hopefully others will too. |
This PR is a continuation of the work started in PRs #400 and #409 about issue #393.
I added a
.clang-format
file enforcing Google-style formatting for C files. I also applied this style to Lwt's C codebase.Overall, I'm happy with the way the files were indented, even if I find the 80-column limit too strict sometimes (example).
clang-format
has quite a lot of options and if you have remarks on what the files look like, please go ahead, I should be able to find the right configuration. :)