Stream: git-wasmtime

Topic: wasmtime / PR #7601 add clang format


view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):

rockwotj requested alexcrichton for a review on PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):

rockwotj requested wasmtime-core-reviewers for a review on PR #7601.

view this post on Zulip 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-format binary in GitHub actions, but I am also happy to download a fixed version if we want to pin the version.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 16:58):

rockwotj requested wasmtime-default-reviewers for a review on PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:00):

alexcrichton commented on PR #7601:

Thanks! Would it actually be reasonable to remove .clang-format and leave clang to its defaults? I'm no clang-format expert myself but if it's common to use the default settings I think that'd be reasonable to do here too

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:02):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:04):

rockwotj commented on PR #7601:

Thanks! Would it actually be reasonable to remove .clang-format and leave clang to its defaults? I'm no clang-format expert myself but if it's common to use the default settings I think that'd be reasonable to do here too

I 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:

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:04):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:05):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:18):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

alexcrichton created PR review comment:

Some more doxygen casualties

view this post on Zulip 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

view this post on Zulip 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

view this post on Zulip 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

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

alexcrichton created PR review comment:

Ditto on comment formatting

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

alexcrichton created PR review comment:

Looks like this may be becoming a broken link

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:19):

alexcrichton created PR review comment:

Ditto on formatting

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:39):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj submitted PR review.

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj created PR review comment:

Fixed.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj created PR review comment:

Fixed.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:40):

rockwotj created PR review comment:

Fixed.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):

rockwotj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):

rockwotj created PR review comment:

Had to disable the formating to get this right.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):

rockwotj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):

rockwotj created PR review comment:

Fixed

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):

rockwotj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:41):

rockwotj created PR review comment:

fixed

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:50):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:50):

alexcrichton created PR review comment:

Might need some reformatting here

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 17:50):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 18:00):

rockwotj updated PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 18:01):

rockwotj submitted PR review.

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 18:20):

alexcrichton has enabled auto merge for PR #7601.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 29 2023 at 19:07):

alexcrichton merged PR #7601.


Last updated: Dec 23 2024 at 12:05 UTC