Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58): Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):rockwotj requested alexcrichton for a review on PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):rockwotj requested wasmtime-core-reviewers for a review on PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):rockwotj opened PR #7601 from rockwotj:clang-format to bytecodealliance:main:
Add a .clang-format for the C code in wasmtime.
We choose the WebKit style because out of all the style options available in .clang-format, it was the closes to the existing style of the c-api crate. I am not really interested in tuning the settings more, but more want consistency and to let the tool format instead of it happening manually.
There is also a check added in CI for it, it always runs and should be very fast. It uses the latest installed
clang-formatbinary in GitHub actions, but I am also happy to download a fixed version if we want to pin the version.
Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):rockwotj requested wasmtime-default-reviewers for a review on PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:00):alexcrichton commented on PR #7601:
Thanks! Would it actually be reasonable to remove
.clang-formatand leave clang to its defaults? I'm noclang-formatexpert myself but if it's common to use the default settings I think that'd be reasonable to do here too
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:02):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:04):rockwotj commented on PR #7601:
Thanks! Would it actually be reasonable to remove
.clang-formatand leave clang to its defaults? I'm noclang-formatexpert myself but if it's common to use the default settings I think that'd be reasonable to do here tooI think it defaults to LLVM style, which is fine with me, I generally haven't seen projects do this, but most of my background comes from Google where I have always seen the Google/Chromium style used. I'll remove it :+1:
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:04):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:05):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:18):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
Some more doxygen casualties
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
Oh dear I think the formatting here may be corrupting the doxygen directives
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
The formatting here may be getting a bit confusing too
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
I think the doxygen bits may be corrupted here as well
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
Ditto on comment formatting
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
Looks like this may be becoming a broken link
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):alexcrichton created PR review comment:
Ditto on formatting
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:39):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj created PR review comment:
I just disabled clang format for this file. Let me know if you think it should be on.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj created PR review comment:
Fixed.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj created PR review comment:
Fixed.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):rockwotj created PR review comment:
Fixed.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):rockwotj created PR review comment:
Had to disable the formating to get this right.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):rockwotj created PR review comment:
Fixed
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):rockwotj created PR review comment:
fixed
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:50):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:50):alexcrichton created PR review comment:
Might need some reformatting here
Wasmtime GitHub notifications bot (Nov 29 2023 at 17:50):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 18:00):rockwotj updated PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 18:01):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Nov 29 2023 at 18:01):rockwotj created PR review comment:
Nice catch! Fixed. Thanks for working with me on this.
Wasmtime GitHub notifications bot (Nov 29 2023 at 18:20):alexcrichton has enabled auto merge for PR #7601.
Wasmtime GitHub notifications bot (Nov 29 2023 at 19:07):alexcrichton merged PR #7601.
Last updated: Oct 23 2024 at 20:03 UTC